[DISCUSS] Submitting small PRs rather than massive ones

[Henry Saputra <he...@gmail.com>]

Hi All,

Recently there have been some PRs with massive changes which include
multiple JIRA tickets.

It is getting tougher to review and also to back port changes if needed.

To help reviewers to help review the changes lets try to submit small
but often PRs to make it easier to review.
Not to mention Github UI suffers with diff changes over 200 files and
thousands lines of code changes =)

When committing to ASF git it should be fine to combine one day of
work but PRs should as isolated as possible.

Exception such as new module like Gelly or ML maybe ok, but others
that require changes to the execution flow should be done if smaller
batches if possible.

Thanks,

Henry

Re: [DISCUSS] Submitting small PRs rather than massive ones

[Henry Saputra <he...@gmail.com>]

Yeah, renaming totally will contain large file changes.

On Thu, Mar 19, 2015 at 1:43 AM, Stephan Ewen <se...@apache.org> wrote:
> I like this proposal very much. We should do that as much as possible.
>
> Pull requests with renaming easily add up to many files, it is harder there.
> Am 18.03.2015 19:39 schrieb "Henry Saputra" <he...@gmail.com>:
>
>> Hi All,
>>
>> Recently there have been some PRs with massive changes which include
>> multiple JIRA tickets.
>>
>> It is getting tougher to review and also to back port changes if needed.
>>
>> To help reviewers to help review the changes lets try to submit small
>> but often PRs to make it easier to review.
>> Not to mention Github UI suffers with diff changes over 200 files and
>> thousands lines of code changes =)
>>
>> When committing to ASF git it should be fine to combine one day of
>> work but PRs should as isolated as possible.
>>
>> Exception such as new module like Gelly or ML maybe ok, but others
>> that require changes to the execution flow should be done if smaller
>> batches if possible.
>>
>> Thanks,
>>
>> Henry
>>

Re: [DISCUSS] Submitting small PRs rather than massive ones

[Henry Saputra <he...@gmail.com>]

+1 to  that, Ufuk.

Making JIRA more descriptive and contain design would make it better
to review b4 jumping to the diff in the PRs.

On Thu, Mar 19, 2015 at 2:17 AM, Ufuk Celebi <uc...@apache.org> wrote:
>
> On 19 Mar 2015, at 09:43, Stephan Ewen <se...@apache.org> wrote:
>
>> I like this proposal very much. We should do that as much as possible.
>
> Same here. Makes it also easier to track progress.
>
> (I think this should go hand in hand with better design descriptions in the corresponding JIRAs.)

Re: [DISCUSS] Submitting small PRs rather than massive ones

[Henry Saputra <he...@gmail.com>]

Great suggestion and observation Max.

+1 Yes, we should also splitting PRs into right and logical commits
that will definitely help with review.
Like you said some PRs are just large in nature and splitting it into
pieces may not work well so doing right commits should go hand in hand
with small PRs as necessary.

When I mean by smaller PRs is to make sure one PR try to solve one
logical issue at a time.
If we can split it into smaller PRs, by designing your solution
propoerly, it would help history management better. Think of it as one
agile sprint story =)

- Henry



On Thu, Mar 19, 2015 at 2:27 AM, Maximilian Michels <mx...@apache.org> wrote:
> I agree with you, Henry. Reviewing hundreds of changes class files is a
> difficult and a nearly impossible task to do exhaustive.
>
> However, splitting up pull requests also has some drawbacks. For example,
> discussions and comments are also split up and harder to keep up with.
> Also, pull requests might depend on other pull requests.
>
> Therefore, I would advise to make use of Gits power and split up pull
> requests into as many logical commits as possible. Individual commits can
> be reviewed just like individual pull requests. The advantage being that
> they can build on each other.
>
> Max
>
> On Thu, Mar 19, 2015 at 10:17 AM, Ufuk Celebi <uc...@apache.org> wrote:
>
>>
>> On 19 Mar 2015, at 09:43, Stephan Ewen <se...@apache.org> wrote:
>>
>> > I like this proposal very much. We should do that as much as possible.
>>
>> Same here. Makes it also easier to track progress.
>>
>> (I think this should go hand in hand with better design descriptions in
>> the corresponding JIRAs.)

Re: [DISCUSS] Submitting small PRs rather than massive ones

[Maximilian Michels <mx...@apache.org>]

I agree with you, Henry. Reviewing hundreds of changes class files is a
difficult and a nearly impossible task to do exhaustive.

However, splitting up pull requests also has some drawbacks. For example,
discussions and comments are also split up and harder to keep up with.
Also, pull requests might depend on other pull requests.

Therefore, I would advise to make use of Gits power and split up pull
requests into as many logical commits as possible. Individual commits can
be reviewed just like individual pull requests. The advantage being that
they can build on each other.

Max

On Thu, Mar 19, 2015 at 10:17 AM, Ufuk Celebi <uc...@apache.org> wrote:

>
> On 19 Mar 2015, at 09:43, Stephan Ewen <se...@apache.org> wrote:
>
> > I like this proposal very much. We should do that as much as possible.
>
> Same here. Makes it also easier to track progress.
>
> (I think this should go hand in hand with better design descriptions in
> the corresponding JIRAs.)

Re: [DISCUSS] Submitting small PRs rather than massive ones

[Ufuk Celebi <uc...@apache.org>]

On 19 Mar 2015, at 09:43, Stephan Ewen <se...@apache.org> wrote:

> I like this proposal very much. We should do that as much as possible.

Same here. Makes it also easier to track progress.

(I think this should go hand in hand with better design descriptions in the corresponding JIRAs.)

Re: [DISCUSS] Submitting small PRs rather than massive ones

[Stephan Ewen <se...@apache.org>]

I like this proposal very much. We should do that as much as possible.

Pull requests with renaming easily add up to many files, it is harder there.
Am 18.03.2015 19:39 schrieb "Henry Saputra" <he...@gmail.com>:

> Hi All,
>
> Recently there have been some PRs with massive changes which include
> multiple JIRA tickets.
>
> It is getting tougher to review and also to back port changes if needed.
>
> To help reviewers to help review the changes lets try to submit small
> but often PRs to make it easier to review.
> Not to mention Github UI suffers with diff changes over 200 files and
> thousands lines of code changes =)
>
> When committing to ASF git it should be fine to combine one day of
> work but PRs should as isolated as possible.
>
> Exception such as new module like Gelly or ML maybe ok, but others
> that require changes to the execution flow should be done if smaller
> batches if possible.
>
> Thanks,
>
> Henry
>