Thanks @Kenn, awesome explanation! Following option (1) now.
On Thu, Jul 20, 2017 at 10:14 AM, Kenneth Knowles <k...@google.com.invalid> wrote: > On Wed, Jul 19, 2017 at 9:35 PM, Mingmin Xu <mingm...@gmail.com> 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 <k...@google.com.invalid> > > 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 <jiang...@gmail.com> 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/ > > > > apache/beam/sdk/io/gcp/datastore/DatastoreV1Test.java > > > > > > > > On Wed, Jul 19, 2017 at 2:52 PM, Mingmin Xu <mingm...@gmail.com> > > wrote: > > > > > > > > > Hi there, > > > > > > > > > > It seems branch DSL_SQL is broken after #3553 > > > > > <https://github.com/apache/beam/pull/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 > > > > > <https://github.com/apache/beam/compare/master...XuMingmin: > > > > > 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 > > > > > > > > > > > > > > > > > > > > -- > > ---- > > Mingmin > > > -- ---- Mingmin
