Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: implement dry-run mode #12

Open
mwichmann opened this issue Aug 6, 2024 · 2 comments
Open

Feature request: implement dry-run mode #12

mwichmann opened this issue Aug 6, 2024 · 2 comments

Comments

@mwichmann
Copy link

mwichmann commented Aug 6, 2024

Hint: maybe enable github Discussions, so folks can ask questions without filing them as issues?

Interested in this, but trying to figure out if it fits in a particular project's workflow. It's hard to tell, if you create a stack of PRs, if the maintainer can merge individual ones as s/he finishes with them through the web UI "as I've always done", or if it has to be done all in one go from the cli (presumably by the submitter, since others are unlikely to have the branches locally in the right form). There's a hint at the latter in the workflow description with this line:

instead of merging them you should use land.

It's also not entirely clear how the workflow works once submit happens and reviewers start asking for changes:

If we need to make changes to any of the PRs (e.g. to address the reviewfeedback), we simply amend the desired changes to the appropriate git commits and run submit again.

That implies force-pushing, which is generally considered a no-no.

If needed, we can rearrange commits or add new ones.

Does that imply new PRs will be created too? Or will they be included in existing branches/PRs?

To come down to something resembling a suggestion/feature request - while some of us aren't really sure what's going on under the covers and whether it will be okay for our particular needs, could the project implement a dry-run mode that shows what would be done without actually making the live changes?

@ZolotukhinM
Copy link
Collaborator

ZolotukhinM commented Aug 7, 2024

Awesome questions, let me try to cover them! I'll also check if I can enable discussions (thanks for the suggestion!), but in the meanwhile I'll reply here:

if you create a stack of PRs, if the maintainer can merge individual ones as s/he finishes with them through the web UI "as I've always done", or if it has to be done all in one go from the cli

The PRs in the created stack will be chained in a sense that only for the bottom PR the base branch (the branch where the PR would be merged to) is main. For the second PR in the stack the base branch would be the head branch of the 1st (bottom) PR. For the 3rd PR the base branch will be the head branch of the 2nd PR, etc.

If you use cli and will try to land the whole stack, it will find the bottom one, land it, and rebase the rest - so everything will be correct.

If you use web UI, go to, say, the 3rd PR, and click "merge", then what will happen is that it will be merged into the branch of the 2nd PR - and that is probably not what you want. This is why the recommended way is to use cli. An astute reader would notice that merging through web UI should still work for the bottom PR, and this is indeed correct, so all in all as long as you understand what you're merging and where to, you can use web UI just fine. Other people can land your PRs too (I did that a couple of times, but that said I won't be surprised if when doing that you'd expose some edge cases that's not currently handled correctly - that's a path less frequently taken, so less tested).

One other thing about merging through web UI vs cli - when you use cli before landing the tool also cleans up the commit message a bit. E.g. it removes the stack info (links to other PRs at the top). If you use web UI the default commit message would have those, so you need to edit it manually before landing if you don't want to have them in your git history.

It's also not entirely clear how the workflow works once submit happens and reviewers start asking for changes:
That implies force-pushing, which is generally considered a no-no.

Force pushing to branches that you share with other people is a no-no, but it's usually ok to force push to your own branches. But yes - if that's not something you can allow in your repo, you won't be able to use the tool, it does force-push to branches it creates (but never to main).

If needed, we can rearrange commits or add new ones.

Does that imply new PRs will be created too? Or will they be included in existing branches/PRs?

If a commit already has a PR associated with it, then that PR will be used after reordering as well (when the tool creates a PR for a commit it adds a special line to the commit message that later is used to associate this commit with that PR). If you add new commits to your stack, and they don't have PRs associated with them (read: their commit messages don't have metainfo pointing to an existing PR), then new PRs will be created for such commits.

To come down to something resembling a suggestion/feature request - while some of us aren't really sure what's going on under the covers and whether it will be okay for our particular needs, could the project implement a dry-run mode that shows what would be done without actually making the live changes?

To some extent you can use stack-pr view as a "dry-run". It will show you

  1. what commits are considered part of the stack
  2. which of them are associated with existing PRs and which are not

And that command never mutates anything, so it that sense it's "dry".

Actual dry-run which would show real commands that are going to be executed is tricky to implement here because the commands depend on each other (e.g. until we actually run gh pr we don't know the PR number). But it should be possible to print an approximate plan for commands to be run, if that'd be helpful.

Personally, I recommend creating a toy repo and play with the tool on it - that in my experience is the fastest way to see what the tool does and how it works.

@mwichmann
Copy link
Author

Thanks for those explanations, helps a lot.

Personally, I recommend creating a toy repo and play with the tool on it - that in my experience is the fastest way to see what the tool does and how it works

I was coming around to that conclusion too, seems a good way to get familiar without upsetting someone else :-)

@ZolotukhinM ZolotukhinM changed the title dry-run mode? Feature request: implement dry-run mode Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants