[Hawkular-dev] [process] tickets and PRs

[Thomas Heute]

On 05/27/2015 06:04 PM, Thomas Segismont wrote:
> I'm opposed to that as a rule.
>
> If a PR has been under works for several days and impacts more than an
> handful of files, then I think it should be created ASAP so that the
> review can start early and be split in small commits.
>
> I think you can't expect a good job from the reviewer when the diff says
> "Showing 60 changed files with 3,593 additions and 2,530 deletions."
> https://github.com/hawkular/hawkular-alerts/pull/50/files

On the other hand reviewing changes affecting the same lines multiple 
times is a waste of time, this is the reason why I squashed most of my 
Kettle->Dist commits as it was just too hard to read through the many 
commits and my mistakes.

I don't think we need a rule, just that we should think about the 
squashing option that Git offers and can help reviewers.

Thomas


>
> Maybe I'm a slacker :)
>
> Le 27/05/2015 13:11, Lucas Ponce a écrit :
>> Let me explain myself better.
>>
>> Or in which situations I think squash is good:
>>
>> - I need to fix a HW-5555.
>> - I checkout master and I create a new branch for HW-5555.
>> - I start to work there:
>> - My commits:
>>     - Start working.
>>     - Adding a couple of things.
>>     - Javadoc typo.
>>     - License agains...
>>
>> - I think this all 4 commits can be squashed without putting in danger anything.
>>
>> - We can send the PR to review without squash, and after that, squash to have a cleaner structure.
>>
>> Again, this can help IMHO.
>>
>> ----- Original Message -----
>>> From: "John Mazzitelli" <mazz at redhat.com>
>>> To: "Discussions around Hawkular development" <hawkular-dev at lists.jboss.org>
>>> Sent: Wednesday, May 27, 2015 1:05:44 PM
>>> Subject: Re: [Hawkular-dev] [process] tickets and PRs
>>>
>>> Uh...I recommend NO squash commits unless you are absolutely sure you know
>>> what you are doing :)
>>>
>>> I can remember we screwed ourselves badly in the beginning of RHQ when we
>>> used squashed commits wrong - the repository got all messed up.
>>>
>>> The only time I do a squash commit is if I immediately destroy the branch it
>>> came from after the squash commit is performed, that way I know I won't
>>> screw anything up.
>>>
>>> ----- Original Message -----
>>>> I also propose to squash commits when it has sense.
>>>>
>>>> Project is growing and for debugging which "commit did that" grouping
>>>> commits
>>>> into a feature/fix/patch can help.
>>>>
>>>> Obviosly there are situations that are not possible, but it can be good to
>>>> start thinking on that.
>>>>
>>>> Just my 2 cents.
>>>> I can remember we screwed ourselves badly in the beginning of RHQ when we used squashed commits wrong - the repository got all messed up.
>>
>>>> ----- Original Message -----
>>>>> From: "Thomas Segismont" <tsegismo at redhat.com>
>>>>> To: hawkular-dev at lists.jboss.org
>>>>> Sent: Wednesday, May 27, 2015 12:12:38 PM
>>>>> Subject: Re: [Hawkular-dev] [process] tickets and PRs
>>>>>
>>>>> Le 27/05/2015 12:06, Thomas Segismont a écrit :
>>>>>> as many of
>>>>>> the commits to the branch have been sent as PR already.
>>>>>
>>>>> Well, we should have sent all commits as part of PR and related to a
>>>>> ticket. That would have made the review process much easier than
>>>>> reviewing PR 228
>>>>> _______________________________________________
>>>>> hawkular-dev mailing list
>>>>> hawkular-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
>>>>>
>>>>
>>>> _______________________________________________
>>>> hawkular-dev mailing list
>>>> hawkular-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
>>>>
>>>
>>> _______________________________________________
>>> hawkular-dev mailing list
>>> hawkular-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
>>>
>>
>> _______________________________________________
>> hawkular-dev mailing list
>> hawkular-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/hawkular-dev
>>
>
> _______________________________________________
> hawkular-dev mailing list
> hawkular-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hawkular-dev
>