<life>won't be able to attend the gov meeting since now DST is finished,
meeting starts at 7PM, exactly while I'm feeding kids</life>
Anyway, I'm personnally for, IIUC, what Oleg somehow expressed here
<https://github.com/jenkinsci/jenkins/pull/1804#issuecomment-149575452>.
That is:
- Like when using Git normally, squashing (well the difference is indeed
you still haven't pushed in that case, anyway) should be encouraged to
provide meaningful commits in the long term.
Meaning: having an initial commit like "Fixes the issue bla blablab...",
then followed by "hack", "fix typo in hack" should really be encouraged to
squash. Cause it doesn't provide any value in the long term and makes it
harder to understand intention.
- IIUC, in that situation, Jesse says that it might be preferrable to
open a new squashed PR superceding the previous one which you'd keep
unsquashed for historical references (?). About that, why not, but as it
makes it more complicated to contribute, and there are risks to have yet
another comments leading to squashing... Well, back to a new
one? Not sure
it would work?
- There can absolutely be many commits in a PR as long as it's a
consistent set related to the PR subject (that is: you shouldn't have
formatting fixes in the middle of commits about some feature/fix related
PR).
My 2 cents.
Cheers
2015-10-28 13:29 GMT+01:00 Oleg Nenashev <[email protected]>:
> Added the topic to the governance meeting agenda.
>
> For instance, suppose I wanted to determine which areas of Jenkins
>> core are the most tricky for contributors to help out with...
>
>
> Well, I suppose we know several hotspots w/o any commit archeology :)
> I understand this idea, but IMO there much more Jenkins users looking into
> GitHub to analyze their regressions, etc. Non-squashed commits impact this
> use-case especially for users, who are not very familiar with Git.
>
> вторник, 20 октября 2015 г., 17:42:45 UTC+3 пользователь Stephen Connolly
> написал:
>>
>> On 20 October 2015 at 15:14, Christopher Orr <[email protected]> wrote:
>> > On 20/10/15 13:26, Stephen Connolly wrote:
>> >> On 20 October 2015 at 12:06, James Nord <[email protected]> wrote:
>> >>>> I have some concerns about mandating a tidy-up (I note that Jesse is
>> >>>> against rewriting the history of a PR branch as that means that
>> GitHub
>> >>>> hides the code review comments)
>> >>>
>> >>>
>> >>> Can you explain this. If I pushed a rebased commit the comments are
>> still
>> >>> there with "commented on an outdated diff" (and if the comment is
>> still
>> >>> applicable to the LOC is still shows) [1]
>> >>> This is no different to comments on a commit being addressed in a
>> future
>> >>> commit and as such are still visible?
>> >>
>> >> Well I will not speak for Jesse, so you would want to check with him
>> >> as to his logic.
>> >>
>> >> I have seen that the GitHub comment tracking feature can be
>> >> unreliable... while it does the right thing and keeps the comments
>> >> with the new rewritten history *most* (say 8 out of 10 times) of the
>> >> time, there are times when it doesn't... so if you rewrite the history
>> >> I cannot trust that all my comments will have been tracked correctly
>> >> (especially if I have more than 10 of them... then there's a good
>> >> chance that 2 of them were mis-tracked... and it's oh so fun trying to
>> >> manually track comments, esp if they get "deleted" and you have to
>> >> switch back to email threads) and I have to re-review all the changes
>> >> to ensure that in rewriting the history you didn't inadvertently
>> >> introduce some other side change
>> >
>> > FWIW (I'm not involved in core dev), what I usually do with GitHub pull
>> > requests is to squash them after the review is complete — I see no need
>> > to pollute git history with ["wip", "cleanup", "cleanup", "address
>> > review comments", "fix nitpicks"] — but I do this entirely locally.
>> >
>> > i.e. I squash commits but don't then force-push that feature branch to
>> > the remote, so I avoid wiping out the "who-committed-what-when"
>> > notifications in the PR, and I avoid screwing up the GitHub comment
>> > tracking.
>> >
>> > That is, I do something like:
>> >
>> > 1. git checkout feature/whatever
>> > 2. git rebase -i HEAD~n
>> > 3. <squash to one or more logical commits>
>> > 4. git checkout master
>> > 5. git merge feature/whatever
>> > 6. git push
>> > 7. Enter "Merged" and click "Comment & close" on the GitHub PR (if the
>> > PR hasn't auto-closed)
>> >
>> > In this case, the commit history and PR comments on GitHub should
>> remain
>> > as they were while the review was in progress.
>>
>> Yes so that fixes one issue but complicates post-hoc analysis of PRs.
>>
>> For instance, suppose I wanted to determine which areas of Jenkins
>> core are the most tricky for contributors to help out with...
>>
>> My initial guess would be to look at the master branch for merge
>> commits, trace each one and count how long the commits being merged
>> were.
>>
>> With the above scheme I no longer have GIT tooling to help and I have
>> to cobble together something involving the GitHub APi as well as
>> GIT... frankenstein's history analyser if you like.
>>
>> If we avoid squashing then this analysis is much easier and just an
>> exercise in command line scripting
>> >
>> > Regards,
>> > Chris
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> Groups "Jenkins Developers" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an email to [email protected].
>> > To view this discussion on the web visit
>> https://groups.google.com/d/msgid/jenkinsci-dev/56264C35.9070705%40orr.me.uk.
>>
>> > For more options, visit https://groups.google.com/d/optout.
>>
> --
> You received this message because you are subscribed to the Google Groups
> "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/jenkinsci-dev/48a6c1f9-18e2-420c-a578-d508d2346abb%40googlegroups.com
> <https://groups.google.com/d/msgid/jenkinsci-dev/48a6c1f9-18e2-420c-a578-d508d2346abb%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
> For more options, visit https://groups.google.com/d/optout.
>
--
Baptiste <Batmat> MATHUS - http://batmat.net
Sauvez un arbre,
Mangez un castor !
--
You received this message because you are subscribed to the Google Groups
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jenkinsci-dev/CANWgJS50Mw417xbEMywmLf5SjTbP4wSdFcCm6idVQMabRxZm9g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
