Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Joshua Root 

On 2016-9-24 06:02 , Ryan Schmidt wrote:


I think we should be able to add build status to commits. Building untrusted 
pull requests is however an entirely different matter. We would have to set up 
a new VM for each build. It's conceivable, but nothing of the kind is currently 
set up or in progress.


Autobuilding pull requests should be a lot safer for base than for 
ports, and that's all we have automated testing set up for at the moment 
anyway. I guess it would still be possible to add malicious code to the 
build system or test suite that performs a DoS or something.


- Josh
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Lawrence Velázquez 
> On Sep 23, 2016, at 2:38 PM, Ryan Schmidt  wrote:
> 
> If you commit directly to master of your fork, then submit a pull
> request, you can't do anything else on master until your pull request
> is accepted. If you make further changes on master they will be
> included in the pull request, which you wouldn't want if they are
> unrelated changes. And once your pull request is merged, then what?
> GitHub will give you a nice "you can now delete your master branch
> because it's been merged" button...

The workflow we recommend should mesh with GitHub's website.
Contributors shouldn't feel like they're fighting the pull request UI or
doing something unorthodox. It sounds like topic/feature branches are
the most natural way to work with pull requests, so that is what we
should recommend.

On a related note, the workflow should reduce openings for error. As
Fred noted, working in a topic branch ensures that updating master
results in a fast-forward merge and shifts conflict resolution to the
point of merging/rebasing the topic branch, which most users might find
less… alarming.

(Advanced Git users will ignore our recommendations and do whatever they
want anyway, which is fine.)

vq
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Sterling Smith 
Clemens,

On Sep 23, 2016, at 12:14PM, Clemens Lang  wrote:

> Hi,
> 
> On Fri, Sep 23, 2016 at 11:56:19AM -0700, Sterling Smith wrote:
>> Being a co-maintainer of a software project hosted on GitHub, where
>> the main developer does not use branches leads to inconsistencies in
>> how his contributions are handled vs other developers.
> 
> What would those inconsistencies be?

The main inconsistency in how his contributions are handled is that other 
developers have their code reviewed during the pull request, whereas he is 
pushing directly to the equivalent of MacPorts/master.  You are right that I 
was conflating fork/master with MacPorts/master.  Sorry.

> What is the difference between the master branch in a fork and an
> arbitrarily named branch based off master in a fork? Can't both be
> rebased against upstream/master in exactly the same way?
> 
>> There should also be a decision on the recommended way to get updates
>> from the latest master, whether that is by merging or rebasing.  I
>> personally like rebasing, but there is a stigma associated with it.
>> Note the possibility of a safer forced push after a rebase with
>> --force-with-lease.  (I didn't look to see if there was already a
>> recommendation.)
> 
> See https://trac.macports.org/wiki/WorkingWithGit#updating. The text
> currently recommends a rebase.

Thanks.

> 
> What's the stigma? The need to force push to your fork?

Since MacPorts is already recommending rebasing, apparently there is no stigma.

> -- 
> Clemens

-Sterling

___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Arno Hautala 
On Fri, Sep 23, 2016 at 4:02 PM, Ryan Schmidt  wrote:
> Building untrusted pull requests is however an entirely different matter

Hah, excellent point. Though now it strikes me what the current
situation is... Hold on, I have to go burn my computers. ;-)

-- 
arno  s  hautala/-|   a...@alum.wpi.edu

pgp b2c9d448
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Rainer Müller 
On 2016-09-23 20:38, Ryan Schmidt wrote:
>> On Sep 23, 2016, at 13:33, Clemens Lang  wrote:
>> 
>> I didn't include this because it's not how I normally work. I
>> usually only create branches for larger changes. I wouldn't be
>> opposed to include it, but I'm probably not the best person to
>> write these docs.
> 
> How can you not work that way?

I usually do the work on my local master. Only when I want to submit a
pull request, I create a new branch starting from upstream/master to
which I cherry-pick or rebase the changes I want to send in the pull
request. I do it this way because I also want to keep all the fixes in
the local copy I use.

Of course that means I need to keep rebasing my local master from
upstream/master.

> If you commit directly to master of your fork, then submit a pull
> request, you can't do anything else on master until your pull request
> is accepted. If you make further changes on master they will be
> included in the pull request, which you wouldn't want if they are
> unrelated changes. And once your pull request is merged, then what?
> GitHub will give you a nice "you can now delete your master branch
> because it's been merged" button...

You could also push from your master to a new remote branch that did not
exist locally:

$ git push origin master:foo-fix

Rainer
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Ryan Schmidt 


> On Sep 23, 2016, at 14:46, Arno Hautala  wrote:
> 
>> On Fri, Sep 23, 2016 at 3:14 PM, Ryan Schmidt  
>> wrote:
>> Seems best to always create a branch before beginning any work, in case you 
>> find out later you have more work you want to submit.
> 
> I use git at work and our workflow is to create a new branch for every
> issue, even if it's as small as a single character typo in a
> documentation file. git clones are really inexpensive so I think the
> advantage really shows itself for smaller changes where you can
> quickly open multiple pull requests.
> 
> We also require that all tests pass before any pull is merged to
> master with the goal that master is never broken. 

Tests are currently broken so we can't enforce such a thing right now. 


> I'm not sure if
> there's any way to integrate the status from the buildbot with Github
> pull requests. Or if there's an easy way to have the buildbot build
> from external branches. It would certainly be nifty if the status of
> each buildbot could be listed next to the pull request for base and
> ports.


I think we should be able to add build status to commits. Building untrusted 
pull requests is however an entirely different matter. We would have to set up 
a new VM for each build. It's conceivable, but nothing of the kind is currently 
set up or in progress. 


___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Arno Hautala 
On Fri, Sep 23, 2016 at 3:14 PM, Ryan Schmidt  wrote:
> Seems best to always create a branch before beginning any work, in case you 
> find out later you have more work you want to submit.

I use git at work and our workflow is to create a new branch for every
issue, even if it's as small as a single character typo in a
documentation file. git clones are really inexpensive so I think the
advantage really shows itself for smaller changes where you can
quickly open multiple pull requests.

We also require that all tests pass before any pull is merged to
master with the goal that master is never broken. I'm not sure if
there's any way to integrate the status from the buildbot with Github
pull requests. Or if there's an easy way to have the buildbot build
from external branches. It would certainly be nifty if the status of
each buildbot could be listed next to the pull request for base and
ports.


-- 
arno  s  hautala/-|   a...@alum.wpi.edu

pgp b2c9d448
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Clemens Lang 
Hi,

On Fri, Sep 23, 2016 at 02:14:25PM -0500, Ryan Schmidt wrote:
> True but I'd like to default to assuming a user will make a valuable
> contribution and will want to make additional contributions in the
> future. And our track record of accepting patches in Trac in a timely
> fashion is not good. Hopefully it will improve once we go to pull
> requests instead of patches but I don't want our failure to timely
> accept a pull request to prevent a user from submitting a second
> unrelated pull request. 

That's a good point.

-- 
Clemens
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Clemens Lang 
Hi,

On Fri, Sep 23, 2016 at 11:56:19AM -0700, Sterling Smith wrote:
> Being a co-maintainer of a software project hosted on GitHub, where
> the main developer does not use branches leads to inconsistencies in
> how his contributions are handled vs other developers.

What would those inconsistencies be?

What is the difference between the master branch in a fork and an
arbitrarily named branch based off master in a fork? Can't both be
rebased against upstream/master in exactly the same way?

> I highly recommend that Clemens move to putting logically separate
> changes in separate topical branches, and avoid developing on master,
> except very tiny changes, e.g. typos in docs.

My point was (as explained in the parallel mail) that I seldom have
multiple logically separate changes at the same time, and just make my
fork's master branch my topic branch for the duration of the pull
request.

> There should also be a decision on the recommended way to get updates
> from the latest master, whether that is by merging or rebasing.  I
> personally like rebasing, but there is a stigma associated with it.
> Note the possibility of a safer forced push after a rebase with
> --force-with-lease.  (I didn't look to see if there was already a
> recommendation.)

See https://trac.macports.org/wiki/WorkingWithGit#updating. The text
currently recommends a rebase.

What's the stigma? The need to force push to your fork?

-- 
Clemens
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Ryan Schmidt 


> On Sep 23, 2016, at 14:02, Clemens Lang  wrote:
> 
> Hi,
> 
> On Fri, Sep 23, 2016 at 01:38:43PM -0500, Ryan Schmidt wrote:
>>> On Sep 23, 2016, at 13:33, Clemens Lang  wrote:
>>> 
>>> I didn't include this because it's not how I normally work. I
>>> usually only create branches for larger changes. I wouldn't be
>>> opposed to include it, but I'm probably not the best person to write
>>> these docs.
>> 
>> If you commit directly to master of your fork, then submit a pull
>> request, you can't do anything else on master until your pull request
>> is accepted. If you make further changes on master they will be
>> included in the pull request, which you wouldn't want if they are
>> unrelated changes.
> 
> For most of the pull requests I've sent on GitHub so far I wanted a
> specific feature or bugfix to be integrated and did not want to make
> other changes, so the branch was just not necessary.
> 
> If you still want to make other changes, you can always work locally,
> too. You can see your local repository as yet another branch.
> 

But I can't submit a pull request for that. 

What has happened to me before is I fork, commit changes to master, pull 
request, upstream does not accept the pull request right away, then I realize I 
have another unrelated change I want to submit. Seems best to always create a 
branch before beginning any work, in case you find out later you have more work 
you want to submit. 


>> And once your pull request is merged, then what? GitHub will give you
>> a nice "you can now delete your master branch because it's been
>> merged" button...
> 
> I usually deleted the entire repository after the PR was merged.

Yes I want to do that too when I'm totally done. GitHub doesn't make this as 
easy though. 



> Maybe this approach will change for MacPorts, but most of our users
> probably don't update 12 different ports in 6 different PRs at the same
> time.

True but I'd like to default to assuming a user will make a valuable 
contribution and will want to make additional contributions in the future. And 
our track record of accepting patches in Trac in a timely fashion is not good. 
Hopefully it will improve once we go to pull requests instead of patches but I 
don't want our failure to timely accept a pull request to prevent a user from 
submitting a second unrelated pull request. 
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Clemens Lang 
Hi,

On Fri, Sep 23, 2016 at 01:38:43PM -0500, Ryan Schmidt wrote:
> > On Sep 23, 2016, at 13:33, Clemens Lang  wrote:
> > 
> > I didn't include this because it's not how I normally work. I
> > usually only create branches for larger changes. I wouldn't be
> > opposed to include it, but I'm probably not the best person to write
> > these docs.
> 
> If you commit directly to master of your fork, then submit a pull
> request, you can't do anything else on master until your pull request
> is accepted. If you make further changes on master they will be
> included in the pull request, which you wouldn't want if they are
> unrelated changes.

For most of the pull requests I've sent on GitHub so far I wanted a
specific feature or bugfix to be integrated and did not want to make
other changes, so the branch was just not necessary.

If you still want to make other changes, you can always work locally,
too. You can see your local repository as yet another branch.

> And once your pull request is merged, then what? GitHub will give you
> a nice "you can now delete your master branch because it's been
> merged" button...

I usually deleted the entire repository after the PR was merged.

Maybe this approach will change for MacPorts, but most of our users
probably don't update 12 different ports in 6 different PRs at the same
time.

-- 
Clemens
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Sterling Smith 
Whether to delete the branch depends largely on how the branch was merged into 
master.  If it was merged as a regular merge (with a merge commit), then there 
is no reason to delete the branch, as a new pull request on the same branch 
(after adding some new commits) will only show any new commits since that 
merge.  On the other hand, if MacPorts decides it likes to squash and merge 
pull requests, then the branch should be deleted both on GitHub and locally, 
otherwise further development on that branch will show both the unsquashed 
commits and the further development in the pull request (although the 
differences shouldn't show up in the files/diff section of the pull request).

Being a co-maintainer of a software project hosted on GitHub, where the main 
developer does not use branches leads to inconsistencies in how his 
contributions are handled vs other developers.  I highly recommend that Clemens 
move to putting logically separate changes in separate topical branches, and 
avoid developing on master, except very tiny changes, e.g. typos in docs.

There should also be a decision on the recommended way to get updates from the 
latest master, whether that is by merging or rebasing.  I personally like 
rebasing, but there is a stigma associated with it.  Note the possibility of a 
safer forced push after a rebase with --force-with-lease.  (I didn't look to 
see if there was already a recommendation.)

-Sterling


On Sep 23, 2016, at 11:38AM, Ryan Schmidt  wrote:

> 
> 
>> On Sep 23, 2016, at 13:33, Clemens Lang  wrote:
>> 
>> I didn't include this because it's not how I normally work. I usually
>> only create branches for larger changes. I wouldn't be opposed to
>> include it, but I'm probably not the best person to write these docs.
> 
> How can you not work that way?
> 
> If you commit directly to master of your fork, then submit a pull request, 
> you can't do anything else on master until your pull request is accepted. If 
> you make further changes on master they will be included in the pull request, 
> which you wouldn't want if they are unrelated changes. And once your pull 
> request is merged, then what? GitHub will give you a nice "you can now delete 
> your master branch because it's been merged" button...
> 
> 
> ___
> macports-dev mailing list
> macports-dev@lists.macosforge.org
> https://lists.macosforge.org/mailman/listinfo/macports-dev

___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Fred Wright 

On Fri, 23 Sep 2016, Clemens Lang wrote:
> On Fri, Sep 23, 2016 at 01:08:38PM -0500, Ryan Schmidt wrote:
> > Something I did not see discussed on this page was that after you
> > fork, and before you start making changes, you should create a branch
> > specific to the work you are doing.
>
> I didn't include this because it's not how I normally work. I usually
> only create branches for larger changes. I wouldn't be opposed to
> include it, but I'm probably not the best person to write these docs.

I always work that way so that I can keep branches inherited from upstream
"pristine", i.e. fast-forwardable.  That's less of an issue for someone
with direct commit access, though even there, working in a branch and
merging separately reduces the collision window for pushes.

Fred Wright
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Ryan Schmidt 


> On Sep 23, 2016, at 13:33, Clemens Lang  wrote:
> 
> I didn't include this because it's not how I normally work. I usually
> only create branches for larger changes. I wouldn't be opposed to
> include it, but I'm probably not the best person to write these docs.

How can you not work that way?

If you commit directly to master of your fork, then submit a pull request, you 
can't do anything else on master until your pull request is accepted. If you 
make further changes on master they will be included in the pull request, which 
you wouldn't want if they are unrelated changes. And once your pull request is 
merged, then what? GitHub will give you a nice "you can now delete your master 
branch because it's been merged" button...


___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Clemens Lang 
On Fri, Sep 23, 2016 at 01:08:38PM -0500, Ryan Schmidt wrote:
> Something I did not see discussed on this page was that after you
> fork, and before you start making changes, you should create a branch
> specific to the work you are doing.

I didn't include this because it's not how I normally work. I usually
only create branches for larger changes. I wouldn't be opposed to
include it, but I'm probably not the best person to write these docs.

In general, I think our "how to pull request" docs aren't very fleshed
out yet; this is intentional, because we don't yet know how we will deal
with pull requests and whether we want to request a specific workflow.

-- 
Clemens
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-09-23 Thread Ryan Schmidt 
Thanks everyone for your work in creating this documentation. 

Something I did not see discussed on this page was that after you fork, and 
before you start making changes, you should create a branch specific to the 
work you are doing. For example if you are updating port foo to a new version, 
make a branch called "update-foo". If you later want to do other unrelated 
work, you create a new branch from master for that. The github pull request 
interface makes it easy to delete your branch after it's been merged, which 
only works if you made a branch to begin with. 


___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-08-22 Thread MacPorts 

Hi Clemens,

On 21.08.2016 17:28, Clemens Lang wrote:

Thanks for the section on setting up commiter name and email address, I
had forgot about this.


you are welcome. I saw the page and that was the first thing hitting my 
eye,

which is why I started to give that wiki page some polish. :-)


+Additionally one should define a few (not necessarily global) presets 
for working with your clone of the MacPorts repository:

+{{{
+$ git config --global push.default nothing
+$ git config --global branch.autosetuprebase always
+$ git config --global core.excludesfile ~/.gitignore_global
+$ git config --global commit.template ~/.git-commit-template
+}}}


Well, I chose to suggest these, as they were recommended by the KDE 
community.




I do not think we should recommend setting these variables for the
following reasons:


Let's see...



 - Users don't understand they're doing here. I want our developers
   to know what and why things are happening with Git, not serve them a
   "here's how you ought to do it" recipe.


So, than we need to explain what these settings do. I am strongly 
opposing the
concept of giving just orders to our port maintainers without educating 
them about
why things were set up the way they are. These settings make sense 
because they
prevent bad things which can happen easily with git. I always favour(ed) 
Mercurial
over it, but it was decided against... so we need to make sure that the 
basic settings

are chosen such that they prevent evil.


 - This is also the reason why the "Fetching the latest changes" 
section

   I wrote yesterday is so long: Developers should understand what
   happens when they run these commands, not just be told "run this". I
   think we should move branch.autosetuprebase to the end of the
   "Fetching the latest changes" section where we tell them that 
instead

   of the --rebase flag, they can also set this option. Also, setting
   this option globally is bad; other projects might not want this
   behavior.


As I wrote above that section I am aware of that those settings might 
not be
wanted as global ones, but I actually have them set as global in most of 
my

setups, to tell you the truth.


 - push.default nothing may be a good idea, but I would only put it as 
a

   recommendation in a section on committing and pushing changes for
   people that want to make sure they're not accidentially pushing
   something they don't want to push.


I think it is a good thing, as it always forces the maintainers to think 
about
which branch they really want to target. That's an extra level of 
precaution.




 - I do not think we need .gitignore_global; We already converted our
   svn:excludes into .gitignores in the repositories and we've always
   left the configuration of svn clients to our developers. If they
   think this is necessary, they can figure it out on their own.


OK, fine with me.


 - I also do not think we should recommend a git commit template for 
the

   same reason. Developers with Subversion have always been free to
   configure their own client and while a commit template is possible
   with Subversion, we did not recommend it there either.


Well, a commit template I definitely favour, because quite some times 
you
do find commits to the MacPorts repo which do not adhere to the 
standards
defined on our Wiki or in the Guide. A template would guide the 
committers
to not to forget some rules of the game. But, if you don't want to annoy 
our

committers with too many rules then so be it. :)

Greets,
Marko

___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev

Re: [MacPorts] WorkingWithGit modified

2016-08-21 Thread Clemens Lang 
Hi Marko,

On Sun, Aug 21, 2016 at 06:55:25AM +, MacPorts wrote:
> Page "WorkingWithGit" was changed by m...@macports.org
> Diff URL: 
> 
> Revision 20
> Comment: Add basic setup info based on KDE's configuration hints
> Changes:
> ---8<--8<--8<--8<--8<--8<--8<--8<
> Index: WorkingWithGit
> =
> --- WorkingWithGit (version: 19)
> +++ WorkingWithGit (version: 20)

Thanks for the section on setting up commiter name and email address, I
had forgot about this.

> +Additionally one should define a few (not necessarily global) presets for 
> working with your clone of the MacPorts repository:
> +{{{
> +$ git config --global push.default nothing
> +$ git config --global branch.autosetuprebase always
> +$ git config --global core.excludesfile ~/.gitignore_global
> +$ git config --global commit.template ~/.git-commit-template
> +}}}

I do not think we should recommend setting these variables for the
following reasons:

 - Users don't understand what they're doing here. I want our developers
   to know what and why things are happening with Git, not serve them a
   "here's how you ought to do it" recipe.
 - This is also the reason why the "Fetching the latest changes" section
   I wrote yesterday is so long: Developers should understand what
   happens when they run these commands, not just be told "run this". I
   think we should move branch.autosetuprebase to the end of the
   "Fetching the latest changes" section where we tell them that instead
   of the --rebase flag, they can also set this option. Also, setting
   this option globally is bad; other projects might not want this
   behavior.
 - push.default nothing may be a good idea, but I would only put it as a
   recommendation in a section on committing and pushing changes for
   people that want to make sure they're not accidentially pushing
   something they don't want to push.
 - I do not think we need .gitignore_global; We already converted our
   svn:excludes into .gitignores in the repositories and we've always
   left the configuration of svn clients to our developers. If they
   think this is necessary, they can figure it out on their own.
 - I also do not think we should recommend a git commit template for the
   same reason. Developers with Subversion have always been free to
   configure their own client and while a commit template is possible
   with Subversion, we did not recommend it there either.

What do you think?

-- 
Clemens
___
macports-dev mailing list
macports-dev@lists.macosforge.org
https://lists.macosforge.org/mailman/listinfo/macports-dev