Usually we ask developers to squash commits when CR already is passed and
PR has +1 from commiter. It is helpful during the batch-commit process then.
Of course it is related to only additional changes, which is added during
CR process.
If PR originally involves several commits to divide some logic in context
of one Jira and PR, these commits could be merged separately.
Also I would like to add some additional check-style for Drill to prevent
comments during CR about missed/redundant spaces and indentation.
On Wed, Sep 26, 2018 at 5:34 AM Boaz Ben-Zvi wrote:
> More on splitting a PR into multiple commits - link [1] below shows
> how to take the last commit and break it (thanks Hanumath.)
>
> I just practiced this method on a PR (1480 - see [2]); this separates
> the actual logic of the change from the less relevant definitions,
> cleanups, etc.
>
> This does require careful manual work from the developer; like if two
> changes are adjacent (i.e. become a single "hunk"), then you need to
> select the "e" option and edit that "hunk".
>
> An open question: Must we eventually squash those multiple commits, or
> would it work better to keep them apart committed into the master ?
>
> Thanks,
>
> Boaz
>
> [1]
>
> https://stackoverflow.com/questions/1440050/how-to-split-last-commit-into-two-in-git/1440200
>
> [2] https://github.com/apache/drill/pull/1480/commits
>
> On 9/24/18 1:49 PM, Jyothsna Reddy wrote:
> > Notes from the Hangout session
> >
> > Attendes:
> > Jyothsna, Boaz, Sorabh, Arina, Bohdan, Ihor, Hanumath, Pritesh, Vitali,
> > Kunal, Robert
> >
> > Interesting thing shared by Boaz : All the minor fragments are assigned
> to
> > Drillbits in round robin fashion and not in a sequential order.
> >
> > Boaz brought up the topic of improving the quality of code reviews:
> > Topic of the Hangout: How do we improve the process of code review?
> >
> > It is highly difficult for the reviewer to do a code review if he/she
> > doesn't know the context and hard to figure out if the PR contains too
> many
> > code changes.
> >
> > Ideas to improve the code review process:
> >
> > - One idea is to break the commits into smaller commits so that each
> > commit is coherent and keeping the refactoring changes in a different
> > commit. But its hard for the developers to separate out into multiple
> > commits if they are too deeply tangled. Although it creates more
> work for
> > developers, it makes reviewers job easier by doing this. This helps
> in
> > finding bugs in earlier stages too.
> > - It would be easier if someone can find ways where Git allows to
> split
> > the commits. Hanumath had tried this earlier.
> > - Mandating check style before code review and it shouldn't be code
> > reviewer's job to point out those.
> > - Bring a reviewer early on into code review process rather than
> dumping
> > a 1 line code changes at a go.
> > - Push smaller commits into master if they make sense.
> > - Do some live code review sessions where external contributors and
> > reviewer can have discussions related to pull requests in a hangout.
> > - Don't squash the commits unless needed.
> > - Reviewers should give full set of comments at one go and there
> > shouldn't be more than 4-5 rounds of code reviews.
> > - Check style should be included for spaces and stuff and developers
> > should try to use IntelliJ IDE and should pay attention to the
> warnings.
> > - Its helpful for reviewers if developers provide screenshots of UI
> for
> > UI changes and attach before and after code if changes are made to
> code
> > generators.
> >
> > Please feel free to add ideas to the above incase if you have any ideas
> to
> > improve the code review process.
> >
> >
> > Thank you,
> > Jyothsna
> >
> >
> > On Mon, Sep 17, 2018 at 12:55 PM Jyothsna Reddy
> > wrote:
> >
> >> The Apache Drill Hangout will be held tomorrow at 10:00am PST; please
> let
> >> us know should you have a topic for tomorrow's hangout. We will also ask
> >> for topics at the beginning of the hangout.
> >>
> >> Hangout Link -
> >>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__hangouts.google.com_hangouts_-5F_event_ci4rdiju8bv04a64efj5fedd0lc=DwIBaQ=cskdkSMqhcnjZxdQVpwTXg=7lXQnf0aC8VQ0iMXwVgNHw=9AQiac0o0ILqquFD8t1gtRKb9VgnUsNWPhyNGEa7x4Q=tNdr_LHgocB7NB3XiSCrp296AMXJgG7YHuOaKD95X74=
> >>
> >> Thank you,
> >> Jyothsna
> >>
>
>
