Support non-default base branch about ghstack HOT 7 CLOSED

Sure, I'd accept a PR that adds this. Should be relatively simple to add support for a -B flag here.

from ghstack.

So you run ghstack in detached state and then use the git hash to revisit a commit to make any changes requested on a PR?

Yep. ghstack checkout BLAH also puts you in a detached state

from ghstack.

Not sure if this is the same but I did not run into an issue submitting a stack off a branch. The created PRs seem to be based on the commits on branch rather than what's on origin/master.

I get a cherry-pick conflict on landing when it tries to land a single/all commits from the branch onto origin/master. In our case, we use three main branches: develop, staging and master (for prod). Reviewed PRs are merged first into develop. The stack here was on top of develop.

$ git symbolic-ref --short HEAD
$ git checkout origin/master
Note: switching to 'origin/master'.
.....
HEAD is now at bf6716e 29 for master
$ git cherry-pick origin/gh/thorbenprimke/1/orig
error: could not apply 333905e... 30
hint: after resolving the conflicts, mark the corrected paths

Based on the above logs, looks like that it should work with a non-default branch for landing if either that specific branch name is provided or the logic could implicitly use the remote tracking branch. I'm gonna dig into the code and see if I can make it work.

Also thanks for creating ghstack. I've been stacking with Phabricator for a long time and it's just a missing feature with GH.

from ghstack.

I looked through the code and based on some hacking, it looks like just default_branch needs to be updated to use the desired branch. Without making the change to submit, I ran into an error with landing.

Personally I could see this work best by implicitly using the tracking branch to override the GH default_branch. However, there is remote_name is the config so perhaps it also could be reasonable to override what's set in GH's default_branch with an option in the ghstack config.

@ezyang - thoughts? You mentioned to use a -B option above.

from ghstack.

@thorbenprimke I think a -B option is obviously a good idea and would accept a patch for it. I'm a little less keen on a ghstackrc, but mostly because it seems like if you had a lot of developers using ghstack on the same repo, they would probably also want to override GH's default_branch too, but everyone would have to individually set the configuration to get it to work out. I mean, it's not the worst thing to add, I just don't think it is the best UX.

Working of an implicit tracking branch is pretty reasonable, but it's a bit tricky to get right. What probably should happen is that at the time you initially create a ghstack, we should record what branch you were based off of, and then permanently work off that (otherwise, when the branch advances, you can't actually tell what branch you were intending to land on, because ghstack doesn't keep any metadata about what original base you wanted to merge into). This actually wouldn't work for my personal workflow (I always work in detached state; well, maybe it would work if you were willing to look for origin/ branches to use as base) but I wouldn't use this feature anyway, so my personal workflow wouldn't matter.

from ghstack.

Yea, makes sense in regards to every person on the repo having to override it via the config. In our project we actually have develop set as the default base. So probably even for us the -B option or implicit use of the tracking branch would make more sense for branch specific fixes to staging or master as they are less frequent.

As far as adding the base information, I suppose it would have to be added to each commit in a stack (since partial stack can be landed) similar to the gh-stack-source-id and PR link. Seems like a reasonable trade off to support this.

So you run ghstack in detached state and then use the git hash to revisit a commit to make any changes requested on a PR?

from ghstack.

I'm not going to do the implicit branch thing unless someone convinces me it's worth it

from ghstack.