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.

Sign up for GitHub

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

Jump to bottom

Add reply w/ Reviewed-by tag of subset of patches from a series #80

Open
wants to merge 4 commits into
base: unstable
Choose a base branch
from
Open

Add reply w/ Reviewed-by tag of subset of patches from a series #80

wants to merge 4 commits into from

Conversation

davidbtadokoro
Copy link
Collaborator

@davidbtadokoro davidbtadokoro commented Oct 30, 2024 •
edited
Loading

PR Commits

  • First: Implement "raw" functionality to reply to a subset of patches from a series. It is "raw" in the sense that it doesn't implement a way to toggle the reply to all patches (which was the previous behavior) and doesn't update the UI component for this functionality
  • Second: Implement the key bind uppercase R to toggle the reply to all patches
  • Third: Update the UI to reflect the changes introduced by the third and fourth commits
  • Fourth: Make UI display the list of patches that were replied with/ the tag as well, giving precedence to those over the staged ones and displaying them in a different color.

Closes: #78

Example

Selecting patches 1, 5, and 7 from a series with 29 patches, in which patches 0, 3, 7, and 11 have already been replied with Reviewed-by.

2024-11-11-134637_1894x1027_scrot

Steps to test this change

  1. Sync your local repo with the latest unstable
  2. Checkout to the latest commit of the unstable (commit 0bbf9c6)
  3. Fetch and switch to this branch
git fetch https://github.com/davidbtadokoro/patch-hub.git feat-reviewed-by-subset
git checkout FETCH_HEAD
  1. Launch patch-hub w/ cargo run --release
  2. IMPORTANT: --dry-run is enabled by default, but, for sanity, check if it is indeed enabled by going to the configurations screen (F2) and checking the config git send email option.
  3. Consult an arbitrary patchset, preferably a multi-patch series.
  4. To stage a patch to reply w/ the tag, hit lowercase r. You should see the title Preview of the patch preview tab become Preview [REVIEWED-BY]*. You should also see the full subset of patches that have been replied w/ the tag and the ones staged in the Details tab as the entry Reviewed-by: (<patch-number-reviewed>,<patch-number-staged>*,...).
  5. To (un)stage all patches, hit uppercase R.
  6. To consolidate the reply of the staged patches, hit ENTER. This should tear the sleek TUI, and display the raw output of the underlying git send-email command.
  7. After returning from the last step, there should be no patch staged to be replied to, no buttons [x] reviewed-by, and the patches that were replied to should be previewed w/ the title Preview [REVIEWED-BY] (without the asterisk) and show in the entry Reviewed-by: of the tab Details.

Any problem report or idea is welcomed, and thank you for testing this feature.

@davidbtadokoro davidbtadokoro force-pushed the feat-reviewed-by-subset branch 2 times, most recently from 176a60b to 5bd3f38 Compare November 6, 2024 05:56
@davidbtadokoro davidbtadokoro marked this pull request as ready for review November 6, 2024 05:56
@davidbtadokoro davidbtadokoro force-pushed the feat-reviewed-by-subset branch from 5bd3f38 to 3c7ab78 Compare November 6, 2024 05:57
@davidbtadokoro davidbtadokoro changed the title [WIP] feat: add reply w/ Reviewed-by to subset of patches Add reply w/ Reviewed-by tag of subset of patches from a series Nov 6, 2024
@davidbtadokoro davidbtadokoro force-pushed the feat-reviewed-by-subset branch 2 times, most recently from 6e7165a to bd967f0 Compare November 8, 2024 16:51
@davidbtadokoro
feat: add reply w/ Reviewed-by to subset of patches
1a36049 
Implement the possibility to only reply to a subset of patches from
a patchset with the `Reviewed-by` tag. Previously, it was only possible
to reply to every patch in the series.

Collaterally, we had to add a field named `patches_to_reply`, which is
a `Vec<usize>`, to `PatchsetDetailsAndActionsState` to keep track of
which patches from the subset to reply with the tag. We also took the
opportunity to make `reviewed_patchsets` from `App`
a `HashMap<String,HashSet<usize>>` instead of
a `HashMap<String,Vec<usize>>`, to ensure uniqueness of patch numbers.

It is important to notice that this commit doesn't implement a reply to
all button, neither updates the UI component for this new changes.

Signed-off-by: David Tadokoro <[email protected]>
@davidbtadokoro
feat: make uppercase R toggle all patches for reply w/ Reviewed-by
f3c5101 
Make hitting the uppercase `R` key result in all patches being
(un)staged, depending if there are patches not staged to be replied (in
this case, stage all) or not (in this case, unstage all).

Signed-off-by: David Tadokoro <[email protected]>
@davidbtadokoro davidbtadokoro force-pushed the feat-reviewed-by-subset branch 2 times, most recently from cc974e9 to bb0d654 Compare November 8, 2024 22:01
@davidbtadokoro
Copy link
Collaborator Author

@rodrigosiqueira, don't know if you've already tested this, but I've pushed a new WIP commit yesterday that changed a bit the display of the patches that have already been reviewed-by and the ones that are staged to be. I've updated the PR description w/ these notions.

@davidbtadokoro davidbtadokoro force-pushed the feat-reviewed-by-subset branch from 43d0753 to fe2acb5 Compare November 11, 2024 16:40
@davidbtadokoro davidbtadokoro changed the base branch from unstable to master November 11, 2024 16:43
@davidbtadokoro davidbtadokoro changed the base branch from master to unstable November 11, 2024 16:43
@davidbtadokoro
feat: update UI for reply w/ Reviewed-by to subset of patches
8c4c457 
Update the UI for the changes introduced by the two last commits.

This results in the button "[x] reviewed-by" in the "Actions" tab having
a display (in the right side) of the number of the patches that are
staged in gray, e.g., if the patches 1, 2, and 7 are staged, in the tab
"Details", there will be an entry like

```
Reviewed-by*: (1,2,7)
```

Signed-off-by: David Tadokoro <[email protected]>
@davidbtadokoro davidbtadokoro force-pushed the feat-reviewed-by-subset branch from fe2acb5 to 0b6d0c7 Compare November 11, 2024 16:55
@davidbtadokoro
feat: show list of patches that already have been Reviewed-by
59b3b1a 
In the last commit, we updated the UI to reflect the new functionality
of replying to a subset of patches from a series w/ the "Reviewed-by"
tag.

Make the line "Reviewed-by:" in the "Details" tab, display information
not only about the staged patches, but the ones recorded as already
replied with the tag, differentiating them by the color and the
appended `*`.

Following the example of the last commit, if a patches 1, 2, and 7 are
staged, and 0, 3, and 10 have already been replied with the tag, the
line will display as

```
Reviewed-by: (0, 1*, 2*, 3, 7*, 10)
```

Closes: kworkflow#78

Signed-off-by: David Tadokoro <[email protected]>
@davidbtadokoro davidbtadokoro force-pushed the feat-reviewed-by-subset branch from 0b6d0c7 to 59b3b1a Compare November 11, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@davidbtadokoro