You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by Zoltan Ivanfi <zi...@cloudera.com> on 2018/03/23 12:22:48 UTC
Merging changes in the GitBox era
Hi,
Parquet was recently migrated to GitBox (thanks, Uwe!). The dev/merge.py
script can be still used by setting PUSH_REMOTE_NAME=apache-github before
invoking it. There is also a new option of merging changes using the GitHub
web UI. I personally continued using the merge script, while others started
using the GitHub web UI. However, I noticed that the latter results in less
clear commit messages/structure:
*git log --oneline* | head
92661a4 Merge pull request #89 from timarmstrong/master
809edf0 Merge pull request #86 from lekv/p323
31a9ddc Update Encodings.md with RLE_DICTIONARY
4d58831 PARQUET-1236: Align version of slf4j-api
2667e08 PARQUET-323: Mark INT96 as deprecated
a64a331 PARQUET-1201: Implement page indexes
9fef1d8 PARQUET-1197: Log rat failures
6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96 type
2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED encodings
c6d306d PARQUET-1064: Deprecate type-defined sort ordering for INTERVAL
type.
*git log --oneline --graph* | head -n 15
* 92661a4 Merge pull request #89 from timarmstrong/master
|\
| * 31a9ddc Update Encodings.md with RLE_DICTIONARY
* | 809edf0 Merge pull request #86 from lekv/p323
|\ \
| |/
|/|
| * 2667e08 PARQUET-323: Mark INT96 as deprecated
* | 4d58831 PARQUET-1236: Align version of slf4j-api
|/
* a64a331 PARQUET-1201: Implement page indexes
* 9fef1d8 PARQUET-1197: Log rat failures
* 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96 type
* 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED encodings
* c6d306d PARQUET-1064: Deprecate type-defined sort ordering for INTERVAL
type.
I would like to ask the developer community: What do you think about the
commits resulting from merging using the GitHub web UI? Are we fine with
these or should we come up with some a policy about how to merge?
Thanks,
Zoltan
Re: Merging changes in the GitBox era
Posted by Lars Volker <lv...@cloudera.com>.
Thanks all for the feedback. I created INFRA-16245
<https://issues.apache.org/jira/browse/INFRA-16245> and linked to this
thread.
On Mon, Mar 26, 2018 at 9:49 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
> I'm fine with that as well, I don't have a strong opinion on this one. And
> we can also remove it any time later.
>
> Zoltan
>
> On Mon, Mar 26, 2018 at 6:43 PM Ryan Blue <rb...@netflix.com.invalid>
> wrote:
>
> > I think we can rely on the "Squash and Merge" default to avoid the
> commits
> > that will dirty the commit log. If you have to select "Rebase and Merge"
> to
> > use it, I think it will probably be fine. And it is nice to have the
> > option.
> >
> > rb
> >
> > On Mon, Mar 26, 2018 at 9:26 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
> >
> > > I think we should, otherwise we may accidentally end up with a commit
> > > history full of micro-changes like "Initial commit for feature",
> "Address
> > > reviewer1's comments", "Address reviewer2's comments".
> > >
> > > Zoltan
> > >
> > > On Mon, Mar 26, 2018 at 6:06 PM Lars Volker <lv...@cloudera.com> wrote:
> > >
> > > > I think we only need "Rebase and Merge" for cases where we want to
> > > submit a
> > > > single PR as multiple commits, e.g. if a PR fixes two JIRAs that
> > somehow
> > > > depend on each other but shouldn't be squashed. Those seem a bit
> > > > hypothetical to me. However, "Rebase and Merge" does prevent the kind
> > of
> > > > merge commit that dirties the git log so I figured we might keep it.
> > I'm
> > > > open to disabling it, too, if you feel we should.
> > > >
> > > > On Mon, Mar 26, 2018 at 6:01 AM, Zoltan Ivanfi <zi...@cloudera.com>
> > wrote:
> > > >
> > > > > To answer my second question: Just tried "Squash and Merge" and I
> > could
> > > > > edit the commit message. Looks good.
> > > > >
> > > > > Zoltan
> > > > >
> > > > > On Mon, Mar 26, 2018 at 2:26 PM Zoltan Ivanfi <zi...@cloudera.com>
> > wrote:
> > > > >
> > > > > > +1 and thanks for taking care of this. I would like to have 2
> > > questions
> > > > > > though:
> > > > > >
> > > > > > - Do we need "Rebase and Merge"?
> > > > > > - What will be the commit message of a "Squash and Merge"?
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Zoltan
> > > > > >
> > > > > >
> > > > > > On Sun, Mar 25, 2018 at 11:45 AM Uwe L. Korn <uw...@xhochy.com>
> > > wrote:
> > > > > >
> > > > > >> +1
> > > > > >>
> > > > > >> On Fri, Mar 23, 2018, at 11:57 PM, Ryan Blue wrote:
> > > > > >> > +1
> > > > > >> >
> > > > > >> > On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <
> > > wesmckinn@gmail.com
> > > > >
> > > > > >> wrote:
> > > > > >> >
> > > > > >> > > +1
> > > > > >> > >
> > > > > >> > > On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <
> lv@cloudera.com
> > >
> > > > > wrote:
> > > > > >> > > > I checked with the Infra team and they can disable some of
> > the
> > > > > >> options
> > > > > >> > > for
> > > > > >> > > > us. Before opening a Jira I'd like to get some more
> feedback
> > > > here.
> > > > > >> The
> > > > > >> > > > options are:
> > > > > >> > > >
> > > > > >> > > > - Disable Merge commits
> > > > > >> > > > - Keep "Squash and Merge" and "Rebase and Merge"
> > > > > >> > > > - Keep "Squash and Merge" as the default. This is what
> > our
> > > > > >> dev/merge
> > > > > >> > > > script does, too.
> > > > > >> > > > - Do this for all the parquet-* repos
> > > > > >> > > >
> > > > > >> > > > Please leave a +1 if you think that's a good idea. Let me
> > know
> > > > if
> > > > > >> you'd
> > > > > >> > > > like to have a formal vote.
> > > > > >> > > >
> > > > > >> > > > Cheers, Lars
> > > > > >> > > >
> > > > > >> > > > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <
> > > > > wesmckinn@gmail.com>
> > > > > >> > > wrote:
> > > > > >> > > >
> > > > > >> > > >> You may be able to ask ASF Infra to disallow merge
> commits
> > in
> > > > the
> > > > > >> > > >> GitHub UI to prevent this from happening
> > > > > >> > > >>
> > > > > >> > > >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <
> > > lv@cloudera.com
> > > > >
> > > > > >> wrote:
> > > > > >> > > >> > Both of these merges are on me, I wasn't aware aware
> that
> > > > they
> > > > > >> would
> > > > > >> > > show
> > > > > >> > > >> > up like this. I personally prefer the format provided
> by
> > > the
> > > > > >> > > dev/merge.py
> > > > > >> > > >> > script.
> > > > > >> > > >> >
> > > > > >> > > >> > Since 2016 Github supports "Rebase and Merge" as a
> second
> > > > > option
> > > > > >> when
> > > > > >> > > >> > submitting PRs:
> > > > > >> > > >> >
> > > > > >>
> > https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> > > > > >> > > >> >
> > > > > >> > > >> > The option is enabled on the parquet-format repository
> > but
> > > I
> > > > > >> don't
> > > > > >> > > have
> > > > > >> > > >> > access to the project configuration to check whether we
> > can
> > > > > >> disable
> > > > > >> > > the
> > > > > >> > > >> > others and make "Rebase and Merge" the default.
> > > > > >> > > >> >
> > > > > >> > > >> > Apologies for messing up the commit log. Please let me
> > know
> > > > if
> > > > > >> you'd
> > > > > >> > > like
> > > > > >> > > >> > me to clean them up with a force-push.
> > > > > >> > > >> >
> > > > > >> > > >> > Cheers, Lars
> > > > > >> > > >> >
> > > > > >> > > >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <
> > > > > zi@cloudera.com>
> > > > > >> > > wrote:
> > > > > >> > > >> >
> > > > > >> > > >> >> Hi,
> > > > > >> > > >> >>
> > > > > >> > > >> >> Parquet was recently migrated to GitBox (thanks,
> Uwe!).
> > > The
> > > > > >> > > dev/merge.py
> > > > > >> > > >> >> script can be still used by setting
> > > > > >> PUSH_REMOTE_NAME=apache-github
> > > > > >> > > >> before
> > > > > >> > > >> >> invoking it. There is also a new option of merging
> > changes
> > > > > >> using the
> > > > > >> > > >> GitHub
> > > > > >> > > >> >> web UI. I personally continued using the merge script,
> > > while
> > > > > >> others
> > > > > >> > > >> started
> > > > > >> > > >> >> using the GitHub web UI. However, I noticed that the
> > > latter
> > > > > >> results
> > > > > >> > > in
> > > > > >> > > >> less
> > > > > >> > > >> >> clear commit messages/structure:
> > > > > >> > > >> >>
> > > > > >> > > >> >> *git log --oneline* | head
> > > > > >> > > >> >> 92661a4 Merge pull request #89 from
> timarmstrong/master
> > > > > >> > > >> >> 809edf0 Merge pull request #86 from lekv/p323
> > > > > >> > > >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > > > > >> > > >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> > > > > >> > > >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> > > > > >> > > >> >> a64a331 PARQUET-1201: Implement page indexes
> > > > > >> > > >> >> 9fef1d8 PARQUET-1197: Log rat failures
> > > > > >> > > >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort
> > ordering
> > > > for
> > > > > >> INT96
> > > > > >> > > >> type
> > > > > >> > > >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> > > > > BIT_PACKED
> > > > > >> > > >> encodings
> > > > > >> > > >> >> c6d306d PARQUET-1064: Deprecate type-defined sort
> > ordering
> > > > for
> > > > > >> > > INTERVAL
> > > > > >> > > >> >> type.
> > > > > >> > > >> >>
> > > > > >> > > >> >> *git log --oneline --graph* | head -n 15
> > > > > >> > > >> >> * 92661a4 Merge pull request #89 from
> > > timarmstrong/master
> > > > > >> > > >> >> |\
> > > > > >> > > >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > > > > >> > > >> >> * | 809edf0 Merge pull request #86 from lekv/p323
> > > > > >> > > >> >> |\ \
> > > > > >> > > >> >> | |/
> > > > > >> > > >> >> |/|
> > > > > >> > > >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> > > > > >> > > >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> > > > > >> > > >> >> |/
> > > > > >> > > >> >> * a64a331 PARQUET-1201: Implement page indexes
> > > > > >> > > >> >> * 9fef1d8 PARQUET-1197: Log rat failures
> > > > > >> > > >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort
> > > ordering
> > > > > for
> > > > > >> > > INT96
> > > > > >> > > >> type
> > > > > >> > > >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for
> RLE,
> > > > > >> BIT_PACKED
> > > > > >> > > >> >> encodings
> > > > > >> > > >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort
> > > ordering
> > > > > for
> > > > > >> > > >> INTERVAL
> > > > > >> > > >> >> type.
> > > > > >> > > >> >>
> > > > > >> > > >> >> I would like to ask the developer community: What do
> you
> > > > think
> > > > > >> about
> > > > > >> > > the
> > > > > >> > > >> >> commits resulting from merging using the GitHub web
> UI?
> > > Are
> > > > we
> > > > > >> fine
> > > > > >> > > with
> > > > > >> > > >> >> these or should we come up with some a policy about
> how
> > to
> > > > > >> merge?
> > > > > >> > > >> >>
> > > > > >> > > >> >> Thanks,
> > > > > >> > > >> >>
> > > > > >> > > >> >> Zoltan
> > > > > >> > > >> >>
> > > > > >> > > >>
> > > > > >> > >
> > > > > >> >
> > > > > >> >
> > > > > >> >
> > > > > >> > --
> > > > > >> > Ryan Blue
> > > > > >> > Software Engineer
> > > > > >> > Netflix
> > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
> >
> >
> > --
> > Ryan Blue
> > Software Engineer
> > Netflix
> >
>
Re: Merging changes in the GitBox era
Posted by Zoltan Ivanfi <zi...@cloudera.com>.
I'm fine with that as well, I don't have a strong opinion on this one. And
we can also remove it any time later.
Zoltan
On Mon, Mar 26, 2018 at 6:43 PM Ryan Blue <rb...@netflix.com.invalid> wrote:
> I think we can rely on the "Squash and Merge" default to avoid the commits
> that will dirty the commit log. If you have to select "Rebase and Merge" to
> use it, I think it will probably be fine. And it is nice to have the
> option.
>
> rb
>
> On Mon, Mar 26, 2018 at 9:26 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
>
> > I think we should, otherwise we may accidentally end up with a commit
> > history full of micro-changes like "Initial commit for feature", "Address
> > reviewer1's comments", "Address reviewer2's comments".
> >
> > Zoltan
> >
> > On Mon, Mar 26, 2018 at 6:06 PM Lars Volker <lv...@cloudera.com> wrote:
> >
> > > I think we only need "Rebase and Merge" for cases where we want to
> > submit a
> > > single PR as multiple commits, e.g. if a PR fixes two JIRAs that
> somehow
> > > depend on each other but shouldn't be squashed. Those seem a bit
> > > hypothetical to me. However, "Rebase and Merge" does prevent the kind
> of
> > > merge commit that dirties the git log so I figured we might keep it.
> I'm
> > > open to disabling it, too, if you feel we should.
> > >
> > > On Mon, Mar 26, 2018 at 6:01 AM, Zoltan Ivanfi <zi...@cloudera.com>
> wrote:
> > >
> > > > To answer my second question: Just tried "Squash and Merge" and I
> could
> > > > edit the commit message. Looks good.
> > > >
> > > > Zoltan
> > > >
> > > > On Mon, Mar 26, 2018 at 2:26 PM Zoltan Ivanfi <zi...@cloudera.com>
> wrote:
> > > >
> > > > > +1 and thanks for taking care of this. I would like to have 2
> > questions
> > > > > though:
> > > > >
> > > > > - Do we need "Rebase and Merge"?
> > > > > - What will be the commit message of a "Squash and Merge"?
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Zoltan
> > > > >
> > > > >
> > > > > On Sun, Mar 25, 2018 at 11:45 AM Uwe L. Korn <uw...@xhochy.com>
> > wrote:
> > > > >
> > > > >> +1
> > > > >>
> > > > >> On Fri, Mar 23, 2018, at 11:57 PM, Ryan Blue wrote:
> > > > >> > +1
> > > > >> >
> > > > >> > On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <
> > wesmckinn@gmail.com
> > > >
> > > > >> wrote:
> > > > >> >
> > > > >> > > +1
> > > > >> > >
> > > > >> > > On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv@cloudera.com
> >
> > > > wrote:
> > > > >> > > > I checked with the Infra team and they can disable some of
> the
> > > > >> options
> > > > >> > > for
> > > > >> > > > us. Before opening a Jira I'd like to get some more feedback
> > > here.
> > > > >> The
> > > > >> > > > options are:
> > > > >> > > >
> > > > >> > > > - Disable Merge commits
> > > > >> > > > - Keep "Squash and Merge" and "Rebase and Merge"
> > > > >> > > > - Keep "Squash and Merge" as the default. This is what
> our
> > > > >> dev/merge
> > > > >> > > > script does, too.
> > > > >> > > > - Do this for all the parquet-* repos
> > > > >> > > >
> > > > >> > > > Please leave a +1 if you think that's a good idea. Let me
> know
> > > if
> > > > >> you'd
> > > > >> > > > like to have a formal vote.
> > > > >> > > >
> > > > >> > > > Cheers, Lars
> > > > >> > > >
> > > > >> > > > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <
> > > > wesmckinn@gmail.com>
> > > > >> > > wrote:
> > > > >> > > >
> > > > >> > > >> You may be able to ask ASF Infra to disallow merge commits
> in
> > > the
> > > > >> > > >> GitHub UI to prevent this from happening
> > > > >> > > >>
> > > > >> > > >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <
> > lv@cloudera.com
> > > >
> > > > >> wrote:
> > > > >> > > >> > Both of these merges are on me, I wasn't aware aware that
> > > they
> > > > >> would
> > > > >> > > show
> > > > >> > > >> > up like this. I personally prefer the format provided by
> > the
> > > > >> > > dev/merge.py
> > > > >> > > >> > script.
> > > > >> > > >> >
> > > > >> > > >> > Since 2016 Github supports "Rebase and Merge" as a second
> > > > option
> > > > >> when
> > > > >> > > >> > submitting PRs:
> > > > >> > > >> >
> > > > >>
> https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> > > > >> > > >> >
> > > > >> > > >> > The option is enabled on the parquet-format repository
> but
> > I
> > > > >> don't
> > > > >> > > have
> > > > >> > > >> > access to the project configuration to check whether we
> can
> > > > >> disable
> > > > >> > > the
> > > > >> > > >> > others and make "Rebase and Merge" the default.
> > > > >> > > >> >
> > > > >> > > >> > Apologies for messing up the commit log. Please let me
> know
> > > if
> > > > >> you'd
> > > > >> > > like
> > > > >> > > >> > me to clean them up with a force-push.
> > > > >> > > >> >
> > > > >> > > >> > Cheers, Lars
> > > > >> > > >> >
> > > > >> > > >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <
> > > > zi@cloudera.com>
> > > > >> > > wrote:
> > > > >> > > >> >
> > > > >> > > >> >> Hi,
> > > > >> > > >> >>
> > > > >> > > >> >> Parquet was recently migrated to GitBox (thanks, Uwe!).
> > The
> > > > >> > > dev/merge.py
> > > > >> > > >> >> script can be still used by setting
> > > > >> PUSH_REMOTE_NAME=apache-github
> > > > >> > > >> before
> > > > >> > > >> >> invoking it. There is also a new option of merging
> changes
> > > > >> using the
> > > > >> > > >> GitHub
> > > > >> > > >> >> web UI. I personally continued using the merge script,
> > while
> > > > >> others
> > > > >> > > >> started
> > > > >> > > >> >> using the GitHub web UI. However, I noticed that the
> > latter
> > > > >> results
> > > > >> > > in
> > > > >> > > >> less
> > > > >> > > >> >> clear commit messages/structure:
> > > > >> > > >> >>
> > > > >> > > >> >> *git log --oneline* | head
> > > > >> > > >> >> 92661a4 Merge pull request #89 from timarmstrong/master
> > > > >> > > >> >> 809edf0 Merge pull request #86 from lekv/p323
> > > > >> > > >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > > > >> > > >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> > > > >> > > >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> > > > >> > > >> >> a64a331 PARQUET-1201: Implement page indexes
> > > > >> > > >> >> 9fef1d8 PARQUET-1197: Log rat failures
> > > > >> > > >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort
> ordering
> > > for
> > > > >> INT96
> > > > >> > > >> type
> > > > >> > > >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> > > > BIT_PACKED
> > > > >> > > >> encodings
> > > > >> > > >> >> c6d306d PARQUET-1064: Deprecate type-defined sort
> ordering
> > > for
> > > > >> > > INTERVAL
> > > > >> > > >> >> type.
> > > > >> > > >> >>
> > > > >> > > >> >> *git log --oneline --graph* | head -n 15
> > > > >> > > >> >> * 92661a4 Merge pull request #89 from
> > timarmstrong/master
> > > > >> > > >> >> |\
> > > > >> > > >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > > > >> > > >> >> * | 809edf0 Merge pull request #86 from lekv/p323
> > > > >> > > >> >> |\ \
> > > > >> > > >> >> | |/
> > > > >> > > >> >> |/|
> > > > >> > > >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> > > > >> > > >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> > > > >> > > >> >> |/
> > > > >> > > >> >> * a64a331 PARQUET-1201: Implement page indexes
> > > > >> > > >> >> * 9fef1d8 PARQUET-1197: Log rat failures
> > > > >> > > >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort
> > ordering
> > > > for
> > > > >> > > INT96
> > > > >> > > >> type
> > > > >> > > >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> > > > >> BIT_PACKED
> > > > >> > > >> >> encodings
> > > > >> > > >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort
> > ordering
> > > > for
> > > > >> > > >> INTERVAL
> > > > >> > > >> >> type.
> > > > >> > > >> >>
> > > > >> > > >> >> I would like to ask the developer community: What do you
> > > think
> > > > >> about
> > > > >> > > the
> > > > >> > > >> >> commits resulting from merging using the GitHub web UI?
> > Are
> > > we
> > > > >> fine
> > > > >> > > with
> > > > >> > > >> >> these or should we come up with some a policy about how
> to
> > > > >> merge?
> > > > >> > > >> >>
> > > > >> > > >> >> Thanks,
> > > > >> > > >> >>
> > > > >> > > >> >> Zoltan
> > > > >> > > >> >>
> > > > >> > > >>
> > > > >> > >
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > --
> > > > >> > Ryan Blue
> > > > >> > Software Engineer
> > > > >> > Netflix
> > > > >>
> > > > >
> > > >
> > >
> >
>
>
>
> --
> Ryan Blue
> Software Engineer
> Netflix
>
Re: Merging changes in the GitBox era
Posted by Ryan Blue <rb...@netflix.com.INVALID>.
I think we can rely on the "Squash and Merge" default to avoid the commits
that will dirty the commit log. If you have to select "Rebase and Merge" to
use it, I think it will probably be fine. And it is nice to have the option.
rb
On Mon, Mar 26, 2018 at 9:26 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
> I think we should, otherwise we may accidentally end up with a commit
> history full of micro-changes like "Initial commit for feature", "Address
> reviewer1's comments", "Address reviewer2's comments".
>
> Zoltan
>
> On Mon, Mar 26, 2018 at 6:06 PM Lars Volker <lv...@cloudera.com> wrote:
>
> > I think we only need "Rebase and Merge" for cases where we want to
> submit a
> > single PR as multiple commits, e.g. if a PR fixes two JIRAs that somehow
> > depend on each other but shouldn't be squashed. Those seem a bit
> > hypothetical to me. However, "Rebase and Merge" does prevent the kind of
> > merge commit that dirties the git log so I figured we might keep it. I'm
> > open to disabling it, too, if you feel we should.
> >
> > On Mon, Mar 26, 2018 at 6:01 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
> >
> > > To answer my second question: Just tried "Squash and Merge" and I could
> > > edit the commit message. Looks good.
> > >
> > > Zoltan
> > >
> > > On Mon, Mar 26, 2018 at 2:26 PM Zoltan Ivanfi <zi...@cloudera.com> wrote:
> > >
> > > > +1 and thanks for taking care of this. I would like to have 2
> questions
> > > > though:
> > > >
> > > > - Do we need "Rebase and Merge"?
> > > > - What will be the commit message of a "Squash and Merge"?
> > > >
> > > > Thanks,
> > > >
> > > > Zoltan
> > > >
> > > >
> > > > On Sun, Mar 25, 2018 at 11:45 AM Uwe L. Korn <uw...@xhochy.com>
> wrote:
> > > >
> > > >> +1
> > > >>
> > > >> On Fri, Mar 23, 2018, at 11:57 PM, Ryan Blue wrote:
> > > >> > +1
> > > >> >
> > > >> > On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <
> wesmckinn@gmail.com
> > >
> > > >> wrote:
> > > >> >
> > > >> > > +1
> > > >> > >
> > > >> > > On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv...@cloudera.com>
> > > wrote:
> > > >> > > > I checked with the Infra team and they can disable some of the
> > > >> options
> > > >> > > for
> > > >> > > > us. Before opening a Jira I'd like to get some more feedback
> > here.
> > > >> The
> > > >> > > > options are:
> > > >> > > >
> > > >> > > > - Disable Merge commits
> > > >> > > > - Keep "Squash and Merge" and "Rebase and Merge"
> > > >> > > > - Keep "Squash and Merge" as the default. This is what our
> > > >> dev/merge
> > > >> > > > script does, too.
> > > >> > > > - Do this for all the parquet-* repos
> > > >> > > >
> > > >> > > > Please leave a +1 if you think that's a good idea. Let me know
> > if
> > > >> you'd
> > > >> > > > like to have a formal vote.
> > > >> > > >
> > > >> > > > Cheers, Lars
> > > >> > > >
> > > >> > > > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <
> > > wesmckinn@gmail.com>
> > > >> > > wrote:
> > > >> > > >
> > > >> > > >> You may be able to ask ASF Infra to disallow merge commits in
> > the
> > > >> > > >> GitHub UI to prevent this from happening
> > > >> > > >>
> > > >> > > >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <
> lv@cloudera.com
> > >
> > > >> wrote:
> > > >> > > >> > Both of these merges are on me, I wasn't aware aware that
> > they
> > > >> would
> > > >> > > show
> > > >> > > >> > up like this. I personally prefer the format provided by
> the
> > > >> > > dev/merge.py
> > > >> > > >> > script.
> > > >> > > >> >
> > > >> > > >> > Since 2016 Github supports "Rebase and Merge" as a second
> > > option
> > > >> when
> > > >> > > >> > submitting PRs:
> > > >> > > >> >
> > > >> https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> > > >> > > >> >
> > > >> > > >> > The option is enabled on the parquet-format repository but
> I
> > > >> don't
> > > >> > > have
> > > >> > > >> > access to the project configuration to check whether we can
> > > >> disable
> > > >> > > the
> > > >> > > >> > others and make "Rebase and Merge" the default.
> > > >> > > >> >
> > > >> > > >> > Apologies for messing up the commit log. Please let me know
> > if
> > > >> you'd
> > > >> > > like
> > > >> > > >> > me to clean them up with a force-push.
> > > >> > > >> >
> > > >> > > >> > Cheers, Lars
> > > >> > > >> >
> > > >> > > >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <
> > > zi@cloudera.com>
> > > >> > > wrote:
> > > >> > > >> >
> > > >> > > >> >> Hi,
> > > >> > > >> >>
> > > >> > > >> >> Parquet was recently migrated to GitBox (thanks, Uwe!).
> The
> > > >> > > dev/merge.py
> > > >> > > >> >> script can be still used by setting
> > > >> PUSH_REMOTE_NAME=apache-github
> > > >> > > >> before
> > > >> > > >> >> invoking it. There is also a new option of merging changes
> > > >> using the
> > > >> > > >> GitHub
> > > >> > > >> >> web UI. I personally continued using the merge script,
> while
> > > >> others
> > > >> > > >> started
> > > >> > > >> >> using the GitHub web UI. However, I noticed that the
> latter
> > > >> results
> > > >> > > in
> > > >> > > >> less
> > > >> > > >> >> clear commit messages/structure:
> > > >> > > >> >>
> > > >> > > >> >> *git log --oneline* | head
> > > >> > > >> >> 92661a4 Merge pull request #89 from timarmstrong/master
> > > >> > > >> >> 809edf0 Merge pull request #86 from lekv/p323
> > > >> > > >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > > >> > > >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> > > >> > > >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> > > >> > > >> >> a64a331 PARQUET-1201: Implement page indexes
> > > >> > > >> >> 9fef1d8 PARQUET-1197: Log rat failures
> > > >> > > >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering
> > for
> > > >> INT96
> > > >> > > >> type
> > > >> > > >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> > > BIT_PACKED
> > > >> > > >> encodings
> > > >> > > >> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering
> > for
> > > >> > > INTERVAL
> > > >> > > >> >> type.
> > > >> > > >> >>
> > > >> > > >> >> *git log --oneline --graph* | head -n 15
> > > >> > > >> >> * 92661a4 Merge pull request #89 from
> timarmstrong/master
> > > >> > > >> >> |\
> > > >> > > >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > > >> > > >> >> * | 809edf0 Merge pull request #86 from lekv/p323
> > > >> > > >> >> |\ \
> > > >> > > >> >> | |/
> > > >> > > >> >> |/|
> > > >> > > >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> > > >> > > >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> > > >> > > >> >> |/
> > > >> > > >> >> * a64a331 PARQUET-1201: Implement page indexes
> > > >> > > >> >> * 9fef1d8 PARQUET-1197: Log rat failures
> > > >> > > >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort
> ordering
> > > for
> > > >> > > INT96
> > > >> > > >> type
> > > >> > > >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> > > >> BIT_PACKED
> > > >> > > >> >> encodings
> > > >> > > >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort
> ordering
> > > for
> > > >> > > >> INTERVAL
> > > >> > > >> >> type.
> > > >> > > >> >>
> > > >> > > >> >> I would like to ask the developer community: What do you
> > think
> > > >> about
> > > >> > > the
> > > >> > > >> >> commits resulting from merging using the GitHub web UI?
> Are
> > we
> > > >> fine
> > > >> > > with
> > > >> > > >> >> these or should we come up with some a policy about how to
> > > >> merge?
> > > >> > > >> >>
> > > >> > > >> >> Thanks,
> > > >> > > >> >>
> > > >> > > >> >> Zoltan
> > > >> > > >> >>
> > > >> > > >>
> > > >> > >
> > > >> >
> > > >> >
> > > >> >
> > > >> > --
> > > >> > Ryan Blue
> > > >> > Software Engineer
> > > >> > Netflix
> > > >>
> > > >
> > >
> >
>
--
Ryan Blue
Software Engineer
Netflix
Re: Merging changes in the GitBox era
Posted by Zoltan Ivanfi <zi...@cloudera.com>.
I think we should, otherwise we may accidentally end up with a commit
history full of micro-changes like "Initial commit for feature", "Address
reviewer1's comments", "Address reviewer2's comments".
Zoltan
On Mon, Mar 26, 2018 at 6:06 PM Lars Volker <lv...@cloudera.com> wrote:
> I think we only need "Rebase and Merge" for cases where we want to submit a
> single PR as multiple commits, e.g. if a PR fixes two JIRAs that somehow
> depend on each other but shouldn't be squashed. Those seem a bit
> hypothetical to me. However, "Rebase and Merge" does prevent the kind of
> merge commit that dirties the git log so I figured we might keep it. I'm
> open to disabling it, too, if you feel we should.
>
> On Mon, Mar 26, 2018 at 6:01 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
>
> > To answer my second question: Just tried "Squash and Merge" and I could
> > edit the commit message. Looks good.
> >
> > Zoltan
> >
> > On Mon, Mar 26, 2018 at 2:26 PM Zoltan Ivanfi <zi...@cloudera.com> wrote:
> >
> > > +1 and thanks for taking care of this. I would like to have 2 questions
> > > though:
> > >
> > > - Do we need "Rebase and Merge"?
> > > - What will be the commit message of a "Squash and Merge"?
> > >
> > > Thanks,
> > >
> > > Zoltan
> > >
> > >
> > > On Sun, Mar 25, 2018 at 11:45 AM Uwe L. Korn <uw...@xhochy.com> wrote:
> > >
> > >> +1
> > >>
> > >> On Fri, Mar 23, 2018, at 11:57 PM, Ryan Blue wrote:
> > >> > +1
> > >> >
> > >> > On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <wesmckinn@gmail.com
> >
> > >> wrote:
> > >> >
> > >> > > +1
> > >> > >
> > >> > > On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv...@cloudera.com>
> > wrote:
> > >> > > > I checked with the Infra team and they can disable some of the
> > >> options
> > >> > > for
> > >> > > > us. Before opening a Jira I'd like to get some more feedback
> here.
> > >> The
> > >> > > > options are:
> > >> > > >
> > >> > > > - Disable Merge commits
> > >> > > > - Keep "Squash and Merge" and "Rebase and Merge"
> > >> > > > - Keep "Squash and Merge" as the default. This is what our
> > >> dev/merge
> > >> > > > script does, too.
> > >> > > > - Do this for all the parquet-* repos
> > >> > > >
> > >> > > > Please leave a +1 if you think that's a good idea. Let me know
> if
> > >> you'd
> > >> > > > like to have a formal vote.
> > >> > > >
> > >> > > > Cheers, Lars
> > >> > > >
> > >> > > > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <
> > wesmckinn@gmail.com>
> > >> > > wrote:
> > >> > > >
> > >> > > >> You may be able to ask ASF Infra to disallow merge commits in
> the
> > >> > > >> GitHub UI to prevent this from happening
> > >> > > >>
> > >> > > >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv@cloudera.com
> >
> > >> wrote:
> > >> > > >> > Both of these merges are on me, I wasn't aware aware that
> they
> > >> would
> > >> > > show
> > >> > > >> > up like this. I personally prefer the format provided by the
> > >> > > dev/merge.py
> > >> > > >> > script.
> > >> > > >> >
> > >> > > >> > Since 2016 Github supports "Rebase and Merge" as a second
> > option
> > >> when
> > >> > > >> > submitting PRs:
> > >> > > >> >
> > >> https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> > >> > > >> >
> > >> > > >> > The option is enabled on the parquet-format repository but I
> > >> don't
> > >> > > have
> > >> > > >> > access to the project configuration to check whether we can
> > >> disable
> > >> > > the
> > >> > > >> > others and make "Rebase and Merge" the default.
> > >> > > >> >
> > >> > > >> > Apologies for messing up the commit log. Please let me know
> if
> > >> you'd
> > >> > > like
> > >> > > >> > me to clean them up with a force-push.
> > >> > > >> >
> > >> > > >> > Cheers, Lars
> > >> > > >> >
> > >> > > >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <
> > zi@cloudera.com>
> > >> > > wrote:
> > >> > > >> >
> > >> > > >> >> Hi,
> > >> > > >> >>
> > >> > > >> >> Parquet was recently migrated to GitBox (thanks, Uwe!). The
> > >> > > dev/merge.py
> > >> > > >> >> script can be still used by setting
> > >> PUSH_REMOTE_NAME=apache-github
> > >> > > >> before
> > >> > > >> >> invoking it. There is also a new option of merging changes
> > >> using the
> > >> > > >> GitHub
> > >> > > >> >> web UI. I personally continued using the merge script, while
> > >> others
> > >> > > >> started
> > >> > > >> >> using the GitHub web UI. However, I noticed that the latter
> > >> results
> > >> > > in
> > >> > > >> less
> > >> > > >> >> clear commit messages/structure:
> > >> > > >> >>
> > >> > > >> >> *git log --oneline* | head
> > >> > > >> >> 92661a4 Merge pull request #89 from timarmstrong/master
> > >> > > >> >> 809edf0 Merge pull request #86 from lekv/p323
> > >> > > >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > >> > > >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> > >> > > >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> > >> > > >> >> a64a331 PARQUET-1201: Implement page indexes
> > >> > > >> >> 9fef1d8 PARQUET-1197: Log rat failures
> > >> > > >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering
> for
> > >> INT96
> > >> > > >> type
> > >> > > >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> > BIT_PACKED
> > >> > > >> encodings
> > >> > > >> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering
> for
> > >> > > INTERVAL
> > >> > > >> >> type.
> > >> > > >> >>
> > >> > > >> >> *git log --oneline --graph* | head -n 15
> > >> > > >> >> * 92661a4 Merge pull request #89 from timarmstrong/master
> > >> > > >> >> |\
> > >> > > >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > >> > > >> >> * | 809edf0 Merge pull request #86 from lekv/p323
> > >> > > >> >> |\ \
> > >> > > >> >> | |/
> > >> > > >> >> |/|
> > >> > > >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> > >> > > >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> > >> > > >> >> |/
> > >> > > >> >> * a64a331 PARQUET-1201: Implement page indexes
> > >> > > >> >> * 9fef1d8 PARQUET-1197: Log rat failures
> > >> > > >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering
> > for
> > >> > > INT96
> > >> > > >> type
> > >> > > >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> > >> BIT_PACKED
> > >> > > >> >> encodings
> > >> > > >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering
> > for
> > >> > > >> INTERVAL
> > >> > > >> >> type.
> > >> > > >> >>
> > >> > > >> >> I would like to ask the developer community: What do you
> think
> > >> about
> > >> > > the
> > >> > > >> >> commits resulting from merging using the GitHub web UI? Are
> we
> > >> fine
> > >> > > with
> > >> > > >> >> these or should we come up with some a policy about how to
> > >> merge?
> > >> > > >> >>
> > >> > > >> >> Thanks,
> > >> > > >> >>
> > >> > > >> >> Zoltan
> > >> > > >> >>
> > >> > > >>
> > >> > >
> > >> >
> > >> >
> > >> >
> > >> > --
> > >> > Ryan Blue
> > >> > Software Engineer
> > >> > Netflix
> > >>
> > >
> >
>
Re: Merging changes in the GitBox era
Posted by Lars Volker <lv...@cloudera.com>.
I think we only need "Rebase and Merge" for cases where we want to submit a
single PR as multiple commits, e.g. if a PR fixes two JIRAs that somehow
depend on each other but shouldn't be squashed. Those seem a bit
hypothetical to me. However, "Rebase and Merge" does prevent the kind of
merge commit that dirties the git log so I figured we might keep it. I'm
open to disabling it, too, if you feel we should.
On Mon, Mar 26, 2018 at 6:01 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
> To answer my second question: Just tried "Squash and Merge" and I could
> edit the commit message. Looks good.
>
> Zoltan
>
> On Mon, Mar 26, 2018 at 2:26 PM Zoltan Ivanfi <zi...@cloudera.com> wrote:
>
> > +1 and thanks for taking care of this. I would like to have 2 questions
> > though:
> >
> > - Do we need "Rebase and Merge"?
> > - What will be the commit message of a "Squash and Merge"?
> >
> > Thanks,
> >
> > Zoltan
> >
> >
> > On Sun, Mar 25, 2018 at 11:45 AM Uwe L. Korn <uw...@xhochy.com> wrote:
> >
> >> +1
> >>
> >> On Fri, Mar 23, 2018, at 11:57 PM, Ryan Blue wrote:
> >> > +1
> >> >
> >> > On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <we...@gmail.com>
> >> wrote:
> >> >
> >> > > +1
> >> > >
> >> > > On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv...@cloudera.com>
> wrote:
> >> > > > I checked with the Infra team and they can disable some of the
> >> options
> >> > > for
> >> > > > us. Before opening a Jira I'd like to get some more feedback here.
> >> The
> >> > > > options are:
> >> > > >
> >> > > > - Disable Merge commits
> >> > > > - Keep "Squash and Merge" and "Rebase and Merge"
> >> > > > - Keep "Squash and Merge" as the default. This is what our
> >> dev/merge
> >> > > > script does, too.
> >> > > > - Do this for all the parquet-* repos
> >> > > >
> >> > > > Please leave a +1 if you think that's a good idea. Let me know if
> >> you'd
> >> > > > like to have a formal vote.
> >> > > >
> >> > > > Cheers, Lars
> >> > > >
> >> > > > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <
> wesmckinn@gmail.com>
> >> > > wrote:
> >> > > >
> >> > > >> You may be able to ask ASF Infra to disallow merge commits in the
> >> > > >> GitHub UI to prevent this from happening
> >> > > >>
> >> > > >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv...@cloudera.com>
> >> wrote:
> >> > > >> > Both of these merges are on me, I wasn't aware aware that they
> >> would
> >> > > show
> >> > > >> > up like this. I personally prefer the format provided by the
> >> > > dev/merge.py
> >> > > >> > script.
> >> > > >> >
> >> > > >> > Since 2016 Github supports "Rebase and Merge" as a second
> option
> >> when
> >> > > >> > submitting PRs:
> >> > > >> >
> >> https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> >> > > >> >
> >> > > >> > The option is enabled on the parquet-format repository but I
> >> don't
> >> > > have
> >> > > >> > access to the project configuration to check whether we can
> >> disable
> >> > > the
> >> > > >> > others and make "Rebase and Merge" the default.
> >> > > >> >
> >> > > >> > Apologies for messing up the commit log. Please let me know if
> >> you'd
> >> > > like
> >> > > >> > me to clean them up with a force-push.
> >> > > >> >
> >> > > >> > Cheers, Lars
> >> > > >> >
> >> > > >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <
> zi@cloudera.com>
> >> > > wrote:
> >> > > >> >
> >> > > >> >> Hi,
> >> > > >> >>
> >> > > >> >> Parquet was recently migrated to GitBox (thanks, Uwe!). The
> >> > > dev/merge.py
> >> > > >> >> script can be still used by setting
> >> PUSH_REMOTE_NAME=apache-github
> >> > > >> before
> >> > > >> >> invoking it. There is also a new option of merging changes
> >> using the
> >> > > >> GitHub
> >> > > >> >> web UI. I personally continued using the merge script, while
> >> others
> >> > > >> started
> >> > > >> >> using the GitHub web UI. However, I noticed that the latter
> >> results
> >> > > in
> >> > > >> less
> >> > > >> >> clear commit messages/structure:
> >> > > >> >>
> >> > > >> >> *git log --oneline* | head
> >> > > >> >> 92661a4 Merge pull request #89 from timarmstrong/master
> >> > > >> >> 809edf0 Merge pull request #86 from lekv/p323
> >> > > >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> >> > > >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> >> > > >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> >> > > >> >> a64a331 PARQUET-1201: Implement page indexes
> >> > > >> >> 9fef1d8 PARQUET-1197: Log rat failures
> >> > > >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for
> >> INT96
> >> > > >> type
> >> > > >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> BIT_PACKED
> >> > > >> encodings
> >> > > >> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
> >> > > INTERVAL
> >> > > >> >> type.
> >> > > >> >>
> >> > > >> >> *git log --oneline --graph* | head -n 15
> >> > > >> >> * 92661a4 Merge pull request #89 from timarmstrong/master
> >> > > >> >> |\
> >> > > >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> >> > > >> >> * | 809edf0 Merge pull request #86 from lekv/p323
> >> > > >> >> |\ \
> >> > > >> >> | |/
> >> > > >> >> |/|
> >> > > >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> >> > > >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> >> > > >> >> |/
> >> > > >> >> * a64a331 PARQUET-1201: Implement page indexes
> >> > > >> >> * 9fef1d8 PARQUET-1197: Log rat failures
> >> > > >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering
> for
> >> > > INT96
> >> > > >> type
> >> > > >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> >> BIT_PACKED
> >> > > >> >> encodings
> >> > > >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering
> for
> >> > > >> INTERVAL
> >> > > >> >> type.
> >> > > >> >>
> >> > > >> >> I would like to ask the developer community: What do you think
> >> about
> >> > > the
> >> > > >> >> commits resulting from merging using the GitHub web UI? Are we
> >> fine
> >> > > with
> >> > > >> >> these or should we come up with some a policy about how to
> >> merge?
> >> > > >> >>
> >> > > >> >> Thanks,
> >> > > >> >>
> >> > > >> >> Zoltan
> >> > > >> >>
> >> > > >>
> >> > >
> >> >
> >> >
> >> >
> >> > --
> >> > Ryan Blue
> >> > Software Engineer
> >> > Netflix
> >>
> >
>
Re: Merging changes in the GitBox era
Posted by Zoltan Ivanfi <zi...@cloudera.com>.
To answer my second question: Just tried "Squash and Merge" and I could
edit the commit message. Looks good.
Zoltan
On Mon, Mar 26, 2018 at 2:26 PM Zoltan Ivanfi <zi...@cloudera.com> wrote:
> +1 and thanks for taking care of this. I would like to have 2 questions
> though:
>
> - Do we need "Rebase and Merge"?
> - What will be the commit message of a "Squash and Merge"?
>
> Thanks,
>
> Zoltan
>
>
> On Sun, Mar 25, 2018 at 11:45 AM Uwe L. Korn <uw...@xhochy.com> wrote:
>
>> +1
>>
>> On Fri, Mar 23, 2018, at 11:57 PM, Ryan Blue wrote:
>> > +1
>> >
>> > On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <we...@gmail.com>
>> wrote:
>> >
>> > > +1
>> > >
>> > > On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv...@cloudera.com> wrote:
>> > > > I checked with the Infra team and they can disable some of the
>> options
>> > > for
>> > > > us. Before opening a Jira I'd like to get some more feedback here.
>> The
>> > > > options are:
>> > > >
>> > > > - Disable Merge commits
>> > > > - Keep "Squash and Merge" and "Rebase and Merge"
>> > > > - Keep "Squash and Merge" as the default. This is what our
>> dev/merge
>> > > > script does, too.
>> > > > - Do this for all the parquet-* repos
>> > > >
>> > > > Please leave a +1 if you think that's a good idea. Let me know if
>> you'd
>> > > > like to have a formal vote.
>> > > >
>> > > > Cheers, Lars
>> > > >
>> > > > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <we...@gmail.com>
>> > > wrote:
>> > > >
>> > > >> You may be able to ask ASF Infra to disallow merge commits in the
>> > > >> GitHub UI to prevent this from happening
>> > > >>
>> > > >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv...@cloudera.com>
>> wrote:
>> > > >> > Both of these merges are on me, I wasn't aware aware that they
>> would
>> > > show
>> > > >> > up like this. I personally prefer the format provided by the
>> > > dev/merge.py
>> > > >> > script.
>> > > >> >
>> > > >> > Since 2016 Github supports "Rebase and Merge" as a second option
>> when
>> > > >> > submitting PRs:
>> > > >> >
>> https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
>> > > >> >
>> > > >> > The option is enabled on the parquet-format repository but I
>> don't
>> > > have
>> > > >> > access to the project configuration to check whether we can
>> disable
>> > > the
>> > > >> > others and make "Rebase and Merge" the default.
>> > > >> >
>> > > >> > Apologies for messing up the commit log. Please let me know if
>> you'd
>> > > like
>> > > >> > me to clean them up with a force-push.
>> > > >> >
>> > > >> > Cheers, Lars
>> > > >> >
>> > > >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <zi...@cloudera.com>
>> > > wrote:
>> > > >> >
>> > > >> >> Hi,
>> > > >> >>
>> > > >> >> Parquet was recently migrated to GitBox (thanks, Uwe!). The
>> > > dev/merge.py
>> > > >> >> script can be still used by setting
>> PUSH_REMOTE_NAME=apache-github
>> > > >> before
>> > > >> >> invoking it. There is also a new option of merging changes
>> using the
>> > > >> GitHub
>> > > >> >> web UI. I personally continued using the merge script, while
>> others
>> > > >> started
>> > > >> >> using the GitHub web UI. However, I noticed that the latter
>> results
>> > > in
>> > > >> less
>> > > >> >> clear commit messages/structure:
>> > > >> >>
>> > > >> >> *git log --oneline* | head
>> > > >> >> 92661a4 Merge pull request #89 from timarmstrong/master
>> > > >> >> 809edf0 Merge pull request #86 from lekv/p323
>> > > >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
>> > > >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
>> > > >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
>> > > >> >> a64a331 PARQUET-1201: Implement page indexes
>> > > >> >> 9fef1d8 PARQUET-1197: Log rat failures
>> > > >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for
>> INT96
>> > > >> type
>> > > >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
>> > > >> encodings
>> > > >> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
>> > > INTERVAL
>> > > >> >> type.
>> > > >> >>
>> > > >> >> *git log --oneline --graph* | head -n 15
>> > > >> >> * 92661a4 Merge pull request #89 from timarmstrong/master
>> > > >> >> |\
>> > > >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
>> > > >> >> * | 809edf0 Merge pull request #86 from lekv/p323
>> > > >> >> |\ \
>> > > >> >> | |/
>> > > >> >> |/|
>> > > >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
>> > > >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
>> > > >> >> |/
>> > > >> >> * a64a331 PARQUET-1201: Implement page indexes
>> > > >> >> * 9fef1d8 PARQUET-1197: Log rat failures
>> > > >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for
>> > > INT96
>> > > >> type
>> > > >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
>> BIT_PACKED
>> > > >> >> encodings
>> > > >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
>> > > >> INTERVAL
>> > > >> >> type.
>> > > >> >>
>> > > >> >> I would like to ask the developer community: What do you think
>> about
>> > > the
>> > > >> >> commits resulting from merging using the GitHub web UI? Are we
>> fine
>> > > with
>> > > >> >> these or should we come up with some a policy about how to
>> merge?
>> > > >> >>
>> > > >> >> Thanks,
>> > > >> >>
>> > > >> >> Zoltan
>> > > >> >>
>> > > >>
>> > >
>> >
>> >
>> >
>> > --
>> > Ryan Blue
>> > Software Engineer
>> > Netflix
>>
>
Re: Merging changes in the GitBox era
Posted by Zoltan Ivanfi <zi...@cloudera.com>.
+1 and thanks for taking care of this. I would like to have 2 questions
though:
- Do we need "Rebase and Merge"?
- What will be the commit message of a "Squash and Merge"?
Thanks,
Zoltan
On Sun, Mar 25, 2018 at 11:45 AM Uwe L. Korn <uw...@xhochy.com> wrote:
> +1
>
> On Fri, Mar 23, 2018, at 11:57 PM, Ryan Blue wrote:
> > +1
> >
> > On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <we...@gmail.com>
> wrote:
> >
> > > +1
> > >
> > > On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv...@cloudera.com> wrote:
> > > > I checked with the Infra team and they can disable some of the
> options
> > > for
> > > > us. Before opening a Jira I'd like to get some more feedback here.
> The
> > > > options are:
> > > >
> > > > - Disable Merge commits
> > > > - Keep "Squash and Merge" and "Rebase and Merge"
> > > > - Keep "Squash and Merge" as the default. This is what our
> dev/merge
> > > > script does, too.
> > > > - Do this for all the parquet-* repos
> > > >
> > > > Please leave a +1 if you think that's a good idea. Let me know if
> you'd
> > > > like to have a formal vote.
> > > >
> > > > Cheers, Lars
> > > >
> > > > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <we...@gmail.com>
> > > wrote:
> > > >
> > > >> You may be able to ask ASF Infra to disallow merge commits in the
> > > >> GitHub UI to prevent this from happening
> > > >>
> > > >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv...@cloudera.com>
> wrote:
> > > >> > Both of these merges are on me, I wasn't aware aware that they
> would
> > > show
> > > >> > up like this. I personally prefer the format provided by the
> > > dev/merge.py
> > > >> > script.
> > > >> >
> > > >> > Since 2016 Github supports "Rebase and Merge" as a second option
> when
> > > >> > submitting PRs:
> > > >> >
> https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> > > >> >
> > > >> > The option is enabled on the parquet-format repository but I don't
> > > have
> > > >> > access to the project configuration to check whether we can
> disable
> > > the
> > > >> > others and make "Rebase and Merge" the default.
> > > >> >
> > > >> > Apologies for messing up the commit log. Please let me know if
> you'd
> > > like
> > > >> > me to clean them up with a force-push.
> > > >> >
> > > >> > Cheers, Lars
> > > >> >
> > > >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <zi...@cloudera.com>
> > > wrote:
> > > >> >
> > > >> >> Hi,
> > > >> >>
> > > >> >> Parquet was recently migrated to GitBox (thanks, Uwe!). The
> > > dev/merge.py
> > > >> >> script can be still used by setting
> PUSH_REMOTE_NAME=apache-github
> > > >> before
> > > >> >> invoking it. There is also a new option of merging changes using
> the
> > > >> GitHub
> > > >> >> web UI. I personally continued using the merge script, while
> others
> > > >> started
> > > >> >> using the GitHub web UI. However, I noticed that the latter
> results
> > > in
> > > >> less
> > > >> >> clear commit messages/structure:
> > > >> >>
> > > >> >> *git log --oneline* | head
> > > >> >> 92661a4 Merge pull request #89 from timarmstrong/master
> > > >> >> 809edf0 Merge pull request #86 from lekv/p323
> > > >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > > >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> > > >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> > > >> >> a64a331 PARQUET-1201: Implement page indexes
> > > >> >> 9fef1d8 PARQUET-1197: Log rat failures
> > > >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for
> INT96
> > > >> type
> > > >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
> > > >> encodings
> > > >> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
> > > INTERVAL
> > > >> >> type.
> > > >> >>
> > > >> >> *git log --oneline --graph* | head -n 15
> > > >> >> * 92661a4 Merge pull request #89 from timarmstrong/master
> > > >> >> |\
> > > >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > > >> >> * | 809edf0 Merge pull request #86 from lekv/p323
> > > >> >> |\ \
> > > >> >> | |/
> > > >> >> |/|
> > > >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> > > >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> > > >> >> |/
> > > >> >> * a64a331 PARQUET-1201: Implement page indexes
> > > >> >> * 9fef1d8 PARQUET-1197: Log rat failures
> > > >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for
> > > INT96
> > > >> type
> > > >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE,
> BIT_PACKED
> > > >> >> encodings
> > > >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
> > > >> INTERVAL
> > > >> >> type.
> > > >> >>
> > > >> >> I would like to ask the developer community: What do you think
> about
> > > the
> > > >> >> commits resulting from merging using the GitHub web UI? Are we
> fine
> > > with
> > > >> >> these or should we come up with some a policy about how to merge?
> > > >> >>
> > > >> >> Thanks,
> > > >> >>
> > > >> >> Zoltan
> > > >> >>
> > > >>
> > >
> >
> >
> >
> > --
> > Ryan Blue
> > Software Engineer
> > Netflix
>
Re: Merging changes in the GitBox era
Posted by "Uwe L. Korn" <uw...@xhochy.com>.
+1
On Fri, Mar 23, 2018, at 11:57 PM, Ryan Blue wrote:
> +1
>
> On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <we...@gmail.com> wrote:
>
> > +1
> >
> > On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv...@cloudera.com> wrote:
> > > I checked with the Infra team and they can disable some of the options
> > for
> > > us. Before opening a Jira I'd like to get some more feedback here. The
> > > options are:
> > >
> > > - Disable Merge commits
> > > - Keep "Squash and Merge" and "Rebase and Merge"
> > > - Keep "Squash and Merge" as the default. This is what our dev/merge
> > > script does, too.
> > > - Do this for all the parquet-* repos
> > >
> > > Please leave a +1 if you think that's a good idea. Let me know if you'd
> > > like to have a formal vote.
> > >
> > > Cheers, Lars
> > >
> > > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <we...@gmail.com>
> > wrote:
> > >
> > >> You may be able to ask ASF Infra to disallow merge commits in the
> > >> GitHub UI to prevent this from happening
> > >>
> > >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv...@cloudera.com> wrote:
> > >> > Both of these merges are on me, I wasn't aware aware that they would
> > show
> > >> > up like this. I personally prefer the format provided by the
> > dev/merge.py
> > >> > script.
> > >> >
> > >> > Since 2016 Github supports "Rebase and Merge" as a second option when
> > >> > submitting PRs:
> > >> > https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> > >> >
> > >> > The option is enabled on the parquet-format repository but I don't
> > have
> > >> > access to the project configuration to check whether we can disable
> > the
> > >> > others and make "Rebase and Merge" the default.
> > >> >
> > >> > Apologies for messing up the commit log. Please let me know if you'd
> > like
> > >> > me to clean them up with a force-push.
> > >> >
> > >> > Cheers, Lars
> > >> >
> > >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <zi...@cloudera.com>
> > wrote:
> > >> >
> > >> >> Hi,
> > >> >>
> > >> >> Parquet was recently migrated to GitBox (thanks, Uwe!). The
> > dev/merge.py
> > >> >> script can be still used by setting PUSH_REMOTE_NAME=apache-github
> > >> before
> > >> >> invoking it. There is also a new option of merging changes using the
> > >> GitHub
> > >> >> web UI. I personally continued using the merge script, while others
> > >> started
> > >> >> using the GitHub web UI. However, I noticed that the latter results
> > in
> > >> less
> > >> >> clear commit messages/structure:
> > >> >>
> > >> >> *git log --oneline* | head
> > >> >> 92661a4 Merge pull request #89 from timarmstrong/master
> > >> >> 809edf0 Merge pull request #86 from lekv/p323
> > >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> > >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> > >> >> a64a331 PARQUET-1201: Implement page indexes
> > >> >> 9fef1d8 PARQUET-1197: Log rat failures
> > >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96
> > >> type
> > >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
> > >> encodings
> > >> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
> > INTERVAL
> > >> >> type.
> > >> >>
> > >> >> *git log --oneline --graph* | head -n 15
> > >> >> * 92661a4 Merge pull request #89 from timarmstrong/master
> > >> >> |\
> > >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> > >> >> * | 809edf0 Merge pull request #86 from lekv/p323
> > >> >> |\ \
> > >> >> | |/
> > >> >> |/|
> > >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> > >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> > >> >> |/
> > >> >> * a64a331 PARQUET-1201: Implement page indexes
> > >> >> * 9fef1d8 PARQUET-1197: Log rat failures
> > >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for
> > INT96
> > >> type
> > >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
> > >> >> encodings
> > >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
> > >> INTERVAL
> > >> >> type.
> > >> >>
> > >> >> I would like to ask the developer community: What do you think about
> > the
> > >> >> commits resulting from merging using the GitHub web UI? Are we fine
> > with
> > >> >> these or should we come up with some a policy about how to merge?
> > >> >>
> > >> >> Thanks,
> > >> >>
> > >> >> Zoltan
> > >> >>
> > >>
> >
>
>
>
> --
> Ryan Blue
> Software Engineer
> Netflix
Re: Merging changes in the GitBox era
Posted by Ryan Blue <rb...@netflix.com.INVALID>.
+1
On Fri, Mar 23, 2018 at 11:01 AM, Wes McKinney <we...@gmail.com> wrote:
> +1
>
> On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv...@cloudera.com> wrote:
> > I checked with the Infra team and they can disable some of the options
> for
> > us. Before opening a Jira I'd like to get some more feedback here. The
> > options are:
> >
> > - Disable Merge commits
> > - Keep "Squash and Merge" and "Rebase and Merge"
> > - Keep "Squash and Merge" as the default. This is what our dev/merge
> > script does, too.
> > - Do this for all the parquet-* repos
> >
> > Please leave a +1 if you think that's a good idea. Let me know if you'd
> > like to have a formal vote.
> >
> > Cheers, Lars
> >
> > On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <we...@gmail.com>
> wrote:
> >
> >> You may be able to ask ASF Infra to disallow merge commits in the
> >> GitHub UI to prevent this from happening
> >>
> >> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv...@cloudera.com> wrote:
> >> > Both of these merges are on me, I wasn't aware aware that they would
> show
> >> > up like this. I personally prefer the format provided by the
> dev/merge.py
> >> > script.
> >> >
> >> > Since 2016 Github supports "Rebase and Merge" as a second option when
> >> > submitting PRs:
> >> > https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> >> >
> >> > The option is enabled on the parquet-format repository but I don't
> have
> >> > access to the project configuration to check whether we can disable
> the
> >> > others and make "Rebase and Merge" the default.
> >> >
> >> > Apologies for messing up the commit log. Please let me know if you'd
> like
> >> > me to clean them up with a force-push.
> >> >
> >> > Cheers, Lars
> >> >
> >> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <zi...@cloudera.com>
> wrote:
> >> >
> >> >> Hi,
> >> >>
> >> >> Parquet was recently migrated to GitBox (thanks, Uwe!). The
> dev/merge.py
> >> >> script can be still used by setting PUSH_REMOTE_NAME=apache-github
> >> before
> >> >> invoking it. There is also a new option of merging changes using the
> >> GitHub
> >> >> web UI. I personally continued using the merge script, while others
> >> started
> >> >> using the GitHub web UI. However, I noticed that the latter results
> in
> >> less
> >> >> clear commit messages/structure:
> >> >>
> >> >> *git log --oneline* | head
> >> >> 92661a4 Merge pull request #89 from timarmstrong/master
> >> >> 809edf0 Merge pull request #86 from lekv/p323
> >> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> >> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> >> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> >> >> a64a331 PARQUET-1201: Implement page indexes
> >> >> 9fef1d8 PARQUET-1197: Log rat failures
> >> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96
> >> type
> >> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
> >> encodings
> >> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
> INTERVAL
> >> >> type.
> >> >>
> >> >> *git log --oneline --graph* | head -n 15
> >> >> * 92661a4 Merge pull request #89 from timarmstrong/master
> >> >> |\
> >> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> >> >> * | 809edf0 Merge pull request #86 from lekv/p323
> >> >> |\ \
> >> >> | |/
> >> >> |/|
> >> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> >> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> >> >> |/
> >> >> * a64a331 PARQUET-1201: Implement page indexes
> >> >> * 9fef1d8 PARQUET-1197: Log rat failures
> >> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for
> INT96
> >> type
> >> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
> >> >> encodings
> >> >> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
> >> INTERVAL
> >> >> type.
> >> >>
> >> >> I would like to ask the developer community: What do you think about
> the
> >> >> commits resulting from merging using the GitHub web UI? Are we fine
> with
> >> >> these or should we come up with some a policy about how to merge?
> >> >>
> >> >> Thanks,
> >> >>
> >> >> Zoltan
> >> >>
> >>
>
--
Ryan Blue
Software Engineer
Netflix
Re: Merging changes in the GitBox era
Posted by Wes McKinney <we...@gmail.com>.
+1
On Fri, Mar 23, 2018 at 1:56 PM, Lars Volker <lv...@cloudera.com> wrote:
> I checked with the Infra team and they can disable some of the options for
> us. Before opening a Jira I'd like to get some more feedback here. The
> options are:
>
> - Disable Merge commits
> - Keep "Squash and Merge" and "Rebase and Merge"
> - Keep "Squash and Merge" as the default. This is what our dev/merge
> script does, too.
> - Do this for all the parquet-* repos
>
> Please leave a +1 if you think that's a good idea. Let me know if you'd
> like to have a formal vote.
>
> Cheers, Lars
>
> On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <we...@gmail.com> wrote:
>
>> You may be able to ask ASF Infra to disallow merge commits in the
>> GitHub UI to prevent this from happening
>>
>> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv...@cloudera.com> wrote:
>> > Both of these merges are on me, I wasn't aware aware that they would show
>> > up like this. I personally prefer the format provided by the dev/merge.py
>> > script.
>> >
>> > Since 2016 Github supports "Rebase and Merge" as a second option when
>> > submitting PRs:
>> > https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
>> >
>> > The option is enabled on the parquet-format repository but I don't have
>> > access to the project configuration to check whether we can disable the
>> > others and make "Rebase and Merge" the default.
>> >
>> > Apologies for messing up the commit log. Please let me know if you'd like
>> > me to clean them up with a force-push.
>> >
>> > Cheers, Lars
>> >
>> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
>> >
>> >> Hi,
>> >>
>> >> Parquet was recently migrated to GitBox (thanks, Uwe!). The dev/merge.py
>> >> script can be still used by setting PUSH_REMOTE_NAME=apache-github
>> before
>> >> invoking it. There is also a new option of merging changes using the
>> GitHub
>> >> web UI. I personally continued using the merge script, while others
>> started
>> >> using the GitHub web UI. However, I noticed that the latter results in
>> less
>> >> clear commit messages/structure:
>> >>
>> >> *git log --oneline* | head
>> >> 92661a4 Merge pull request #89 from timarmstrong/master
>> >> 809edf0 Merge pull request #86 from lekv/p323
>> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
>> >> 4d58831 PARQUET-1236: Align version of slf4j-api
>> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
>> >> a64a331 PARQUET-1201: Implement page indexes
>> >> 9fef1d8 PARQUET-1197: Log rat failures
>> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96
>> type
>> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
>> encodings
>> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for INTERVAL
>> >> type.
>> >>
>> >> *git log --oneline --graph* | head -n 15
>> >> * 92661a4 Merge pull request #89 from timarmstrong/master
>> >> |\
>> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
>> >> * | 809edf0 Merge pull request #86 from lekv/p323
>> >> |\ \
>> >> | |/
>> >> |/|
>> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
>> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
>> >> |/
>> >> * a64a331 PARQUET-1201: Implement page indexes
>> >> * 9fef1d8 PARQUET-1197: Log rat failures
>> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96
>> type
>> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
>> >> encodings
>> >> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
>> INTERVAL
>> >> type.
>> >>
>> >> I would like to ask the developer community: What do you think about the
>> >> commits resulting from merging using the GitHub web UI? Are we fine with
>> >> these or should we come up with some a policy about how to merge?
>> >>
>> >> Thanks,
>> >>
>> >> Zoltan
>> >>
>>
Re: Merging changes in the GitBox era
Posted by Lars Volker <lv...@cloudera.com>.
I checked with the Infra team and they can disable some of the options for
us. Before opening a Jira I'd like to get some more feedback here. The
options are:
- Disable Merge commits
- Keep "Squash and Merge" and "Rebase and Merge"
- Keep "Squash and Merge" as the default. This is what our dev/merge
script does, too.
- Do this for all the parquet-* repos
Please leave a +1 if you think that's a good idea. Let me know if you'd
like to have a formal vote.
Cheers, Lars
On Fri, Mar 23, 2018 at 8:21 AM, Wes McKinney <we...@gmail.com> wrote:
> You may be able to ask ASF Infra to disallow merge commits in the
> GitHub UI to prevent this from happening
>
> On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv...@cloudera.com> wrote:
> > Both of these merges are on me, I wasn't aware aware that they would show
> > up like this. I personally prefer the format provided by the dev/merge.py
> > script.
> >
> > Since 2016 Github supports "Rebase and Merge" as a second option when
> > submitting PRs:
> > https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
> >
> > The option is enabled on the parquet-format repository but I don't have
> > access to the project configuration to check whether we can disable the
> > others and make "Rebase and Merge" the default.
> >
> > Apologies for messing up the commit log. Please let me know if you'd like
> > me to clean them up with a force-push.
> >
> > Cheers, Lars
> >
> > On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
> >
> >> Hi,
> >>
> >> Parquet was recently migrated to GitBox (thanks, Uwe!). The dev/merge.py
> >> script can be still used by setting PUSH_REMOTE_NAME=apache-github
> before
> >> invoking it. There is also a new option of merging changes using the
> GitHub
> >> web UI. I personally continued using the merge script, while others
> started
> >> using the GitHub web UI. However, I noticed that the latter results in
> less
> >> clear commit messages/structure:
> >>
> >> *git log --oneline* | head
> >> 92661a4 Merge pull request #89 from timarmstrong/master
> >> 809edf0 Merge pull request #86 from lekv/p323
> >> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> >> 4d58831 PARQUET-1236: Align version of slf4j-api
> >> 2667e08 PARQUET-323: Mark INT96 as deprecated
> >> a64a331 PARQUET-1201: Implement page indexes
> >> 9fef1d8 PARQUET-1197: Log rat failures
> >> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96
> type
> >> 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
> encodings
> >> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for INTERVAL
> >> type.
> >>
> >> *git log --oneline --graph* | head -n 15
> >> * 92661a4 Merge pull request #89 from timarmstrong/master
> >> |\
> >> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> >> * | 809edf0 Merge pull request #86 from lekv/p323
> >> |\ \
> >> | |/
> >> |/|
> >> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> >> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> >> |/
> >> * a64a331 PARQUET-1201: Implement page indexes
> >> * 9fef1d8 PARQUET-1197: Log rat failures
> >> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96
> type
> >> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
> >> encodings
> >> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering for
> INTERVAL
> >> type.
> >>
> >> I would like to ask the developer community: What do you think about the
> >> commits resulting from merging using the GitHub web UI? Are we fine with
> >> these or should we come up with some a policy about how to merge?
> >>
> >> Thanks,
> >>
> >> Zoltan
> >>
>
Re: Merging changes in the GitBox era
Posted by Wes McKinney <we...@gmail.com>.
You may be able to ask ASF Infra to disallow merge commits in the
GitHub UI to prevent this from happening
On Fri, Mar 23, 2018 at 11:18 AM, Lars Volker <lv...@cloudera.com> wrote:
> Both of these merges are on me, I wasn't aware aware that they would show
> up like this. I personally prefer the format provided by the dev/merge.py
> script.
>
> Since 2016 Github supports "Rebase and Merge" as a second option when
> submitting PRs:
> https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
>
> The option is enabled on the parquet-format repository but I don't have
> access to the project configuration to check whether we can disable the
> others and make "Rebase and Merge" the default.
>
> Apologies for messing up the commit log. Please let me know if you'd like
> me to clean them up with a force-push.
>
> Cheers, Lars
>
> On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
>
>> Hi,
>>
>> Parquet was recently migrated to GitBox (thanks, Uwe!). The dev/merge.py
>> script can be still used by setting PUSH_REMOTE_NAME=apache-github before
>> invoking it. There is also a new option of merging changes using the GitHub
>> web UI. I personally continued using the merge script, while others started
>> using the GitHub web UI. However, I noticed that the latter results in less
>> clear commit messages/structure:
>>
>> *git log --oneline* | head
>> 92661a4 Merge pull request #89 from timarmstrong/master
>> 809edf0 Merge pull request #86 from lekv/p323
>> 31a9ddc Update Encodings.md with RLE_DICTIONARY
>> 4d58831 PARQUET-1236: Align version of slf4j-api
>> 2667e08 PARQUET-323: Mark INT96 as deprecated
>> a64a331 PARQUET-1201: Implement page indexes
>> 9fef1d8 PARQUET-1197: Log rat failures
>> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96 type
>> 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED encodings
>> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for INTERVAL
>> type.
>>
>> *git log --oneline --graph* | head -n 15
>> * 92661a4 Merge pull request #89 from timarmstrong/master
>> |\
>> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
>> * | 809edf0 Merge pull request #86 from lekv/p323
>> |\ \
>> | |/
>> |/|
>> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
>> * | 4d58831 PARQUET-1236: Align version of slf4j-api
>> |/
>> * a64a331 PARQUET-1201: Implement page indexes
>> * 9fef1d8 PARQUET-1197: Log rat failures
>> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96 type
>> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
>> encodings
>> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering for INTERVAL
>> type.
>>
>> I would like to ask the developer community: What do you think about the
>> commits resulting from merging using the GitHub web UI? Are we fine with
>> these or should we come up with some a policy about how to merge?
>>
>> Thanks,
>>
>> Zoltan
>>
Re: Merging changes in the GitBox era
Posted by Lars Volker <lv...@cloudera.com>.
Both of these merges are on me, I wasn't aware aware that they would show
up like this. I personally prefer the format provided by the dev/merge.py
script.
Since 2016 Github supports "Rebase and Merge" as a second option when
submitting PRs:
https://blog.github.com/2016-09-26-rebase-and-merge-pull-requests/
The option is enabled on the parquet-format repository but I don't have
access to the project configuration to check whether we can disable the
others and make "Rebase and Merge" the default.
Apologies for messing up the commit log. Please let me know if you'd like
me to clean them up with a force-push.
Cheers, Lars
On Fri, Mar 23, 2018 at 5:22 AM, Zoltan Ivanfi <zi...@cloudera.com> wrote:
> Hi,
>
> Parquet was recently migrated to GitBox (thanks, Uwe!). The dev/merge.py
> script can be still used by setting PUSH_REMOTE_NAME=apache-github before
> invoking it. There is also a new option of merging changes using the GitHub
> web UI. I personally continued using the merge script, while others started
> using the GitHub web UI. However, I noticed that the latter results in less
> clear commit messages/structure:
>
> *git log --oneline* | head
> 92661a4 Merge pull request #89 from timarmstrong/master
> 809edf0 Merge pull request #86 from lekv/p323
> 31a9ddc Update Encodings.md with RLE_DICTIONARY
> 4d58831 PARQUET-1236: Align version of slf4j-api
> 2667e08 PARQUET-323: Mark INT96 as deprecated
> a64a331 PARQUET-1201: Implement page indexes
> 9fef1d8 PARQUET-1197: Log rat failures
> 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96 type
> 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED encodings
> c6d306d PARQUET-1064: Deprecate type-defined sort ordering for INTERVAL
> type.
>
> *git log --oneline --graph* | head -n 15
> * 92661a4 Merge pull request #89 from timarmstrong/master
> |\
> | * 31a9ddc Update Encodings.md with RLE_DICTIONARY
> * | 809edf0 Merge pull request #86 from lekv/p323
> |\ \
> | |/
> |/|
> | * 2667e08 PARQUET-323: Mark INT96 as deprecated
> * | 4d58831 PARQUET-1236: Align version of slf4j-api
> |/
> * a64a331 PARQUET-1201: Implement page indexes
> * 9fef1d8 PARQUET-1197: Log rat failures
> * 6e5b78d PARQUET-1065: Deprecate type-defined sort ordering for INT96 type
> * 2696f9e PARQUET-1171: Clarify scope of usage for RLE, BIT_PACKED
> encodings
> * c6d306d PARQUET-1064: Deprecate type-defined sort ordering for INTERVAL
> type.
>
> I would like to ask the developer community: What do you think about the
> commits resulting from merging using the GitHub web UI? Are we fine with
> these or should we come up with some a policy about how to merge?
>
> Thanks,
>
> Zoltan
>