On 12 December 2016 at 10:35, Joshua Root wrote: > On 2016-12-12 20:21 , Rainer Müller wrote: >> On 2016-12-12 02:29, Lawrence Velázquez wrote: >>>> On Dec 6, 2016, at 10:33 AM, Rainer Müller wrote: >>>> >>>> (e) add "Closes: #XYZ" to the commit message >>> >>> When commits close PRs implicitly (by merging the PR branch) instead of >>> explicitly (by using "closes #XYZ" in the message), the PR is remembered >>> internally by GitHub and displayed on the website but is not recorded in our >>> Git repository. If we ever migrate off GitHub, we would presumably lose this >>> information. >>> >>> Should we consider this a problem? >> >> >> I don't think there is any value in preserving all patch iterations of a >> pull request. I would compare this to an initial patch submission in the >> issue tracker, which are often merged in a different form later. Nobody >> will look at the initial patch again or even remember. > > I think the information Larry is referring to is not the actual PR contents,
I don't think the initial patch is relevant either. But the discussion might be. Generally I never open any old tickets, but sometimes when working on a particular port I want to understand why there's a specific patch or some specific line of code, then go through commit history and often find references to trac tickets with an extensive discussions that explains a lot. Having an explicit link to a GitHub PR would certainly help. > but rather the links from commits to PRs. I also think that this information might be useful. When opening tickets on Trac (or if we had issues), we would always provide a link. On GitHub this is only implicit and even now people with local git clients (either command line or gui ones) don't get a link to the PR in case they are interested. The stupid bit is that getting this right is sometimes a bit annoying. I recently opened a pull request with 18 commits. (I wanted to know if anyone had any concerns before merging; eventually I just committed everything and there was zero discussion, but that's not the point.) One doesn't know the PR number in advance, so putting in any number would be pure speculation. On the other hand putting that information in after opening the pull request would require editing all the 18 commit messages. It's not impossible, just a bit annoying. > I don't think this is a big > problem, I don't think it is a big problem either. But it's a good point and I would be in favour of it (but not insisting). > but it's one more reason to open a trac ticket for any change that > involves significant discussion that should be archived. I would argue that you never know in advance how much discussion a pull request would attract. On top of that you would often want to comment on a specific line of code. Opening a ticket on Trac just for "extensive discussion" about pull request will just spread the information to more places and lead to more confusion (I would never know whether to comment on Trac or on GitHub). It is not unrealistic that we will want to or have to migrate one day in the future, there's already quite a bit of the desired functionality missing, but it's the best option we have at the moment (like sourceforge used to be the place-to-go for opensource projects long long time ago). In that case we could still leave the account open and old PRs could remain there (or be migrated and links somehow fixed). This actually reminded me of another potential "problem". In the past we always used the (closes #1234567) syntax which we changed during migration to GIT to absolute links. Do we want to use absolute links for Closes: https://github.com/macports/macports-ports/pull/1234567 as well? Mojca