add patch series #244
Closed
add patch series #244
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Owner
drewdeponte
commented
Oct 19, 2023
drewdeponte
commented
- Add patch series support request_review_branch
- Add patch series support to sync command
- Add patch series support to request-review cmd
- Change iso command args to patch_index_or_range
- Add patch series support to show & int cmds
drewdeponte
force-pushed
the
add_patch_series
branch
3 times, most recently
from
3df9bc1 to
7e0ac02
Compare
Add patch series support request_review_branch so that users would be able to either request review of a single patch or of a patch series. Doing this was suprisingly involved. It required completely changing how we were previously adding patch ids to commits. Before we would effectively rebase the patch stack amending the patch selected with a provided patch id if it didn't have one. Now we take a bit of a different approach. Now we made it part of our cherry picking functionality and as we cherry pick we can optionally add a patch id to any commits missing them. This allows us to simply add_patch_ids() near the beginning of any commands that need to make sure the commits in the stack have patch ids. Note: We have explicitly opted to call add_patch_ids() in these other commands to avoid requiring users to use `gps c`. We deamed this important as a number of users are using other Git tools in concert with gps and we didn't want to force them to use `gps c`. In order implement add_patch_ids() I first had to move the cherry picking functionality over to ps/mod.rs so that I could add a configuration parameter to it add_missing_patch_ids and when enabled handle checking for patch ids as I cherry pick and generate and add any missing ones. This makes sense to now be in ps/mod.rs as the cherry pick methods are now aware of a patch stack concept, patch ids. Once I had these functions I was able to rewrite the request_review_branch() function to support individual patches or a patch series. The general process is the same with a couple differences. 1. use new add_patch_ids() near the top 2. determine the associated branch in a different manner 3. cherry pick the patch series as ranged cherry pick The last major piece of the puzzle was updating the CLI to accept a String for the patch_index_or_range and then implelement a function to parse that String into a PatchIndexRange type. This lets use provide the patch index as <start-index>-<end-index>, e.g. 2-5. [changelog] added: patch series support to request-review-branch cmd <!-- ps-id: f5726cb3-dbbb-4ee9-a50b-1c9482e8f7fe -->
Add patch series support to sync command so that users would be able to sync their patch series. [changelog] added: patch series support to the sync command <!-- ps-id: 198d2e18-8144-4ef5-99e8-2a1b07a306b6 -->
Add patch series support to request-review command so that users would be able to request review for their series of patches. [changelog] added: patch series support to the request-review command <!-- ps-id: 2cad7a7c-40b0-4530-8cd8-56e95cfb2e46 -->
Change iso command args to patch_index_or_range so that it patches our new standard command line interface for dealing with a patch or a series of patches. [changelog] changed: isolate command args to use new patch index or range interface <!-- ps-id: aa73bc98-3e9a-4d32-98f6-74353c71f2a7 -->
Add patch series support to the show & integrate commands so that users
would be able to show & integrate an individual patch or a patch series.
As part of this I also added a new hook to integrate that gets when not
using the --force switch. It is called `integrate_verify` the intent
with it is that you could use it to check the status of a PR from a
remote systems like GitHub to prevent you from integrating. This might
be useful to check the CI status of a PR or Approval status of a PR for
example.
Supporting patch series for the show command was pretty straight
forward. However, the integrate command was another thing. The way we
had it implemented previously wasn't very clear what the distinction
between --force and non-force were. Also all of the checks in the
non-force were previosly written explicitly as if there was one patch
and not a series. So, I effectively restructured and rewrote the entire
thing.
A few things that I determined while designing this.
Similiar with all the other commands the user should have to think as
little as possible about the concept of branches and focus primarily on
the concept of patches. In this context this primarily means that the
state of the patches in patch stack is what should be integrated into
the upstream. So, I decided that we always want to create/replace the
request review branch so that it has the most recent state from the
patch stack prior to the actual push. This is different from the
previous implementation where it only did this when the --force switch
was present.
I also decided that --force effectively means to skip the safety checks
that make sure the patches in the patch stack match the patches in the
remote branch prior to create/replace the request review branch. This is
different than the previous implementation as in the previous
implementation the create/replace the request review branch would only
happen if the --force switch was passed.
The following is an outline of how integrate should conceptually be
working now.
- validate patch indexes are within bounds
- add patch ids
- prompt_for_reassurance (based on config)
- git fetch - update the knowledge of the remotes
- if NOT force
- figure out associated branch
- verify has associated branch, exit with error
- check to make sure patches match between stack & remote
- TODO: in the future: execute hook to verify PR approval & CI status
- create/replace the request review branch
- in fresh case, it creates the branch, in existing case it updates it to the latest state from ps
- verify isolation (verifies cherry-pick cleanly but also verify isolation hook passes)
- publish the patch(es) from local patch branch up to patch stack upstream
- execute integrate post push hook
- optionally (based on config) delete remote if exists request review branch
- optionally (based on config) delete local request review branch
- optionnaly pull (based on config)
[changelog]
added: patch series support to the show command
added: patch series support to the integrate command
added: integrate_verify hook to the integrate command
<!-- ps-id: 09ca28df-b891-41c0-b36b-aedf30f4b83a -->
drewdeponte
force-pushed
the
add_patch_series
branch
from
7e0ac02 to
409f94e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.