Re: [NEED HELP] how to revert a PR in branch DSL_SQL

2017-07-20 Thread Mingmin Xu 
Thanks @Kenn, awesome explanation! Following option (1) now.


On Thu, Jul 20, 2017 at 10:14 AM, Kenneth Knowles 
wrote:

> On Wed, Jul 19, 2017 at 9:35 PM, Mingmin Xu  wrote:
>
> > Merge with conflict is not a good choice to me either as lots of files
> are
> > impacted.
> >
>
> Yes, don't do this. You don't need to. I think reverting the merge commit
> is a good idea, then you can start again on this process.
>
>
> > @Kenn, one more question, what's the point by 'but use git merge', any
> > difference from how it was processed in #3553?
> >
>
> In #3533 all of the commits from master were duplicated. The effect of
> whatever rebase happened was to bulk cherrypick them onto the
> github/pr/3553 branch, and then merge those new commits to DSL_SQL.
>
> Specifically, it looks like something like this happened:
>
> git checkout github/pr/3553
> git rebase github/DSL_SQL
> git push apache DSL_SQL
>
> At this point, all of the commits from master that were in #3553 have been
> copied to new, identical, commits that are based on the DSL_SQL branch.
> Maybe the rebase was on something else but anything that is not already in
> the history of github/pr/3553 will cause the same problem.
>
> Instead do this:
>
> git checkout github/DSL_SQL
> git merge --no-ff github/pr/3553
> git push apache DSL_SQL
>
> This will not duplicate any commits.
>
> I find it helpful to remember that rebase is basically bulk cherrypick. It
> is sort of OK to rebase normal PRs because those commits are "dead" after
> the commits have been cherrypicked to master. But these problems don't
> occur if you don't use rebase or cherrypick.
>
> I could be wrong about the exact sequence that led to the situation, but I
> am pretty certain that reverting the merge commit and then doing a normal
> merge will work.
>
> Kenn
>
> On Wed, Jul 19, 2017 at 7:05 PM, Kenneth Knowles 
> > wrote:
> >
> > > Yes, merging by rebase doesn't work when you have two branches that
> > > interact.
> > >
> > > I checked `git log github/DSL_SQL ^github/master` to understand what is
> > > going on.
> > >
> > > Here are some ideas:
> > >
> > > I think your option (1) is fine, but you should revert and then replay
> > > #3553 but use git merge. This will work because the duplicated commits
> > that
> > > get reverted are not the ones on master. Then you can do any updates
> you
> > > need and propose the merge DSL_SQL -> master.
> > >
> > > Kenn
> > >
> > > On Wed, Jul 19, 2017 at 5:56 PM, Kai Jiang  wrote:
> > >
> > > > Or another option is solve merge conflict. This might not be the best
> > > way.
> > > > Because once master branch has some changes, we still need to do this
> > > same
> > > > way.
> > > >
> > > > I tried merge locally. We could solve conflict files and open PR for
> > > that.
> > > >
> > > > conflict files are:
> > > > both modified:   examples/java/src/main/java/
> > > > org/apache/beam/examples/common/WriteOneFilePerWindow.java
> > > > both modified:   examples/java8/src/main/java/
> > org/apache/beam/examples/
> > > > complete/game/utils/WriteToText.java
> > > > both modified:   runners/core-construction-
> > > java/src/test/java/org/apache/
> > > > beam/runners/core/construction/WriteFilesTranslationTest.java
> > > > both modified:   sdks/java/core/src/main/java/
> org/apache/beam/sdk/io/
> > > > DefaultFilenamePolicy.java
> > > > both modified:   sdks/java/core/src/main/java/
> org/apache/beam/sdk/io/
> > > > FileBasedSink.java
> > > > both modified:   sdks/java/core/src/main/java/
> > > > org/apache/beam/sdk/io/TextIO.
> > > > java
> > > > both modified:   sdks/java/core/src/main/java/
> org/apache/beam/sdk/io/
> > > > WriteFiles.java
> > > > both modified:   sdks/java/core/src/main/java/
> > > org/apache/beam/sdk/values/
> > > > PCollectionViews.java
> > > > both modified:   sdks/java/core/src/test/java/
> org/apache/beam/sdk/io/
> > > > AvroIOTest.java
> > > > both modified:   sdks/java/core/src/test/java/
> org/apache/beam/sdk/io/
> > > > FileBasedSinkTest.java
> > > > both modified:   sdks/java/core/src/test/java/
> org/apache/beam/sdk/io/
> > > > TextIOTest.java
> > > > both modified:   sdks/java/core/src/test/java/
> org/apache/beam/sdk/io/
> > > > WriteFilesTest.java
> > > > both modified:   sdks/java/harness/src/main/java/org/apache/beam/fn/
> > > > harness/control/ProcessBundleHandler.java
> > > > both modified:   sdks/java/harness/src/test/java/org/apache/beam/fn/
> > > > harness/control/ProcessBundleHandlerTest.java
> > > > deleted by us:   sdks/java/harness/src/test/
> > > java/org/apache/beam/runners/
> > > > core/BoundedSourceRunnerTest.java
> > > > both modified:   sdks/java/io/google-cloud-
> platform/src/main/java/org/
> > > > apache/beam/sdk/io/gcp/datastore/DatastoreV1.java
> > > > both modified:   sdks/java/io/google-cloud-
> platform/src/test/java/org/
> > > >

Re: [NEED HELP] how to revert a PR in branch DSL_SQL

2017-07-19 Thread Kai Jiang 
Or another option is solve merge conflict. This might not be the best way.
Because once master branch has some changes, we still need to do this same
way.

I tried merge locally. We could solve conflict files and open PR for that.

conflict files are:
both modified:   examples/java/src/main/java/
org/apache/beam/examples/common/WriteOneFilePerWindow.java
both modified:   examples/java8/src/main/java/org/apache/beam/examples/
complete/game/utils/WriteToText.java
both modified:   runners/core-construction-java/src/test/java/org/apache/
beam/runners/core/construction/WriteFilesTranslationTest.java
both modified:   sdks/java/core/src/main/java/org/apache/beam/sdk/io/
DefaultFilenamePolicy.java
both modified:   sdks/java/core/src/main/java/org/apache/beam/sdk/io/
FileBasedSink.java
both modified:   sdks/java/core/src/main/java/org/apache/beam/sdk/io/TextIO.
java
both modified:   sdks/java/core/src/main/java/org/apache/beam/sdk/io/
WriteFiles.java
both modified:   sdks/java/core/src/main/java/org/apache/beam/sdk/values/
PCollectionViews.java
both modified:   sdks/java/core/src/test/java/org/apache/beam/sdk/io/
AvroIOTest.java
both modified:   sdks/java/core/src/test/java/org/apache/beam/sdk/io/
FileBasedSinkTest.java
both modified:   sdks/java/core/src/test/java/org/apache/beam/sdk/io/
TextIOTest.java
both modified:   sdks/java/core/src/test/java/org/apache/beam/sdk/io/
WriteFilesTest.java
both modified:   sdks/java/harness/src/main/java/org/apache/beam/fn/
harness/control/ProcessBundleHandler.java
both modified:   sdks/java/harness/src/test/java/org/apache/beam/fn/
harness/control/ProcessBundleHandlerTest.java
deleted by us:   sdks/java/harness/src/test/java/org/apache/beam/runners/
core/BoundedSourceRunnerTest.java
both modified:   sdks/java/io/google-cloud-platform/src/main/java/org/
apache/beam/sdk/io/gcp/datastore/DatastoreV1.java
both modified:   sdks/java/io/google-cloud-platform/src/test/java/org/
apache/beam/sdk/io/gcp/datastore/DatastoreV1Test.java

On Wed, Jul 19, 2017 at 2:52 PM, Mingmin Xu  wrote:

> Hi there,
>
> It seems branch DSL_SQL is broken after #3553
> , as I cannot create a PR to
> master branch with error message '*Can’t automatically merge.*'.
>
> Googled and find two solutions:
> 1.  submit a revert PR with Git
> https://stackoverflow.com/questions/2389361/undo-a-git-merge
> -that-hasnt-been-pushed-yet/6217372#6217372
>
>
> I follow this way and here are the steps: (need to adjust target for real
> case)
>   a. the revert PR https://github.com/XuMingmin/beam/pull/14;
>   b. branch which has applied PR in a)
> https://github.com/XuMingmin/beam/tree/revert_3553_test;
>   c.  Now I can create a PR from XuMingmin/beam/revert_3553_test to
> apache/beam/master, see link
>  revert_3553_test?expand=1>
> ;
>
> 2. reverting a pull request in Github
> https://help.github.com/articles/reverting-a-pull-request/
> This is a feature in Github, I cannot see the '*revert*' button maybe
> because of permission.
>
> For both the two, I think #3553 need to redo in the end.
>
> Any suggestion which is the right way to go, or any other options?
>
> --
> 
> Mingmin
>