-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add reply w/ Reviewed-by
tag of subset of patches from a series
#80
Merged
davidbtadokoro
merged 5 commits into
kworkflow:unstable
from
davidbtadokoro:feat-reviewed-by-subset
Jan 29, 2025
Merged
Add reply w/ Reviewed-by
tag of subset of patches from a series
#80
davidbtadokoro
merged 5 commits into
kworkflow:unstable
from
davidbtadokoro:feat-reviewed-by-subset
Jan 29, 2025
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
0e52741
to
8f7a6dc
Compare
176a60b
to
5bd3f38
Compare
5bd3f38
to
3c7ab78
Compare
Reviewed-by
tag of subset of patches from a series
cc974e9
to
bb0d654
Compare
@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. |
43d0753
to
fe2acb5
Compare
0b6d0c7
to
59b3b1a
Compare
8f1157d
to
7055b09
Compare
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. Closes: kworkflow#78 Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
…tion 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 <davidbtadokoro@usp.br>
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 <davidbtadokoro@usp.br>
The `b4` tool [1] applies reply trailers when fecthing patches, i.e., if a patch has already been replied with the tags "Reviewed-by", "Tested-by", etc. (referred by `b4` as "code-review trailers") when the the tool consults the thread, it adds them to the returned .mbox. This enables us to present useful information to users about the review status of the patch. To make the "Patchset Details and Actions" screen more useful, extract these trailers when initializing the screen and present them in the "Details" tab as a line like Reviewed-by: 2 | Tested-by: 0 | Acked-by: 1 [1]: https://b4.docs.kernel.org/en/latest/ Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
Following the previous commit, where we added functionality related to showing code-review trailer stats, i.e., how many "Reviewed-by", "Tested-by", and "Acked-by" a specific patch has, add a pop-up for the "Patchset Details and Actions" that further details from whom these trailers came. This is done by binding the combination `Ctrl+t` to the creation of the newly implemented `PopUp` type `ReviewTrailerPopUp` and binding it with `app.popup`. Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>
7055b09
to
d3e99e6
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.
PR Commits
R
to toggle the reply to all patchesCloses: #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
.Steps to test this change
patch-hub
w/cargo run --release
--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 configgit send email option
.r
. You should see the titlePreview
of the patch preview tab becomePreview [REVIEWED-BY]*
. You should also see the full subset of patches that have been replied w/ the tag and the ones staged in theDetails
tab as the entryReviewed-by: (<patch-number-reviewed>,<patch-number-staged>*,...)
.R
.ENTER
. This should tear the sleek TUI, and display the raw output of the underlyinggit send-email
command.[x] reviewed-by
, and the patches that were replied to should be previewed w/ the titlePreview [REVIEWED-BY]
(without the asterisk) and show in the entryReviewed-by:
of the tabDetails
.Any problem report or idea is welcomed, and thank you for testing this feature.