forgejo-contrib-forgejo-cli-issues-146.md
Watch20
Star402
Fork
You've already forked forgejo-cli
CodeIssues 44Pull requests 8Releases 11Packages 2WikiActivity
Open
opened 2024-12-12 20:10:40 +01:00 by Cyborus · 13 comments
Cyborus commented 2024-12-12 20:10:40 +01:00
Member
Copy link
Commands to create and view PR reviews
fj pr review [id]
startcommentstatussubmit [--approve OR --deny]pr review start would create a local file where a list of review comments will be added to with pr review comment. fj pr review submit would then upload that review to Forgejo.
Commands to create and view PR reviews - fj pr review [id] - start - comment - status - submit [--approve OR --deny] pr review start would create a local file where a list of review comments will be added to with pr review comment. fj pr review submit would then upload that review to Forgejo.
❤️ 2
Cyborus added this to the v0.3.0 milestone 2024-12-12 20:10:40 +01:00
Cyborus added the Kind/Feature label 2024-12-12 20:10:40 +01:00
Cyborus removed this from the v0.3.0 milestone 2025-05-05 19:34:21 +02:00
alwayscoding commented 2026-03-13 10:41:43 +01:00
Copy link
If no one's working on this atm I'm happy to give this a try 🙂
If no one's working on this atm I'm happy to give this a try 🙂
👍 1 ❤️ 2
Fluffinity commented 2026-06-20 22:18:32 +02:00
Contributor
Copy link
@alwayscoding were you able to make progress? Otherwise we can try to slowly tackle this one together. There are a lot of details to take care of for this feature, so none of us really needs to get everything right themselves :)
@alwayscoding were you able to make progress? Otherwise we can try to slowly tackle this one together. There are a lot of details to take care of for this feature, so none of us really needs to get everything right themselves :)
alwayscoding commented 2026-06-21 08:38:28 +02:00
Copy link
Hey @Fluffinity and everyone, apologies for the radio silence. I did make some progress, unfortunately got a little stuck in how to approach it with saving PR comments to a file and then marking as reviewed :(. I do have a branch here with some attempts. Happy to work together or for you to take the changes and progress them, whichever works 🙂
Hey @Fluffinity and everyone, apologies for the radio silence. I did make some progress, unfortunately got a little stuck in how to approach it with saving PR comments to a file and then marking as reviewed :(. I do have a branch here with some attempts. Happy to work together or for you to take the changes and progress them, whichever works 🙂
Fluffinity commented 2026-06-21 12:01:47 +02:00
Contributor
Copy link
We can totally work together on that one, if you have the time. My first steps will be to study the review API endpoints, just to check against your current implementation. Then I should also have a better idea on how to persist the review state until submission. Giving the user a nice looking output and workflow would be the last step for me, once the core functionality already works. Who knows, maybe you also quickly find a solution to the persistence problem, now that you had this mental break from it
We can totally work together on that one, if you have the time. My first steps will be to study the review API endpoints, just to check against your current implementation. Then I should also have a better idea on how to persist the review state until submission. Giving the user a nice looking output and workflow would be the last step for me, once the core functionality already works. Who knows, maybe you also quickly find a solution to the persistence problem, now that you had this mental break from it
alwayscoding commented 2026-06-21 14:35:18 +02:00
Copy link
Sounds good, happy to help out where I can! From memory, Forgejo doesn't have a draft PR state, so any comments would need to be stored on disk. Of course, there's also the issue of remembering that a PR has started on a specific branch / PR. So let's try to scope how far the memory should go.
In terms of APIs that will be relevant for this feature, creating a review is what would be called to mark a PR review as done. So how about for logic
?
Hope this provides a starting point
Sounds good, happy to help out where I can! From memory, Forgejo doesn't have a draft PR state, so any comments would need to be stored on disk. Of course, there's also the issue of remembering that a PR has started on a specific branch / PR. So let's try to scope how far the memory should go. In terms of APIs that will be relevant for this feature, creating a review is what would be called to mark a PR review as done. So how about for logic - (Optional) check if there is a review in progress - Create a PR: Store a file on disk, forgejo-cli does have a helper for that - Comments etc - Finish review: send comments if any, or approve ? Hope this provides a starting point
Fluffinity commented 2026-06-21 17:30:06 +02:00
Contributor
Copy link
Right now the most confusing part for me to figure out is figuring out the requirements for the body in POST: /repos/{owner}/{repo}/pulls/{index}/reviews and POST: /repos/{owner}/{repo}/pulls/{index}/reviews/{id}. When creating a new review there is a top-level body for the API call and a body for each comment. The latter is straightforward, but what exactly is the top-level body supposed to be? Then there is also the ReviewStateType, that is just a plain string, making it unclear what we are supposed to put in there, Maybe I can find out more by checking the Forgejo source code, but if anyone of you knows the expected inputs, please let me know
Right now the most confusing part for me to figure out is figuring out the requirements for the body in POST: /repos/{owner}/{repo}/pulls/{index}/reviews and POST: /repos/{owner}/{repo}/pulls/{index}/reviews/{id}. When creating a new review there is a top-level body for the API call and a body for each comment. The latter is straightforward, but what exactly is the top-level body supposed to be? Then there is also the ReviewStateType, that is just a plain string, making it unclear what we are supposed to put in there, Maybe I can find out more by checking the Forgejo source code, but if anyone of you knows the expected inputs, please let me know
Fluffinity commented 2026-06-21 17:35:16 +02:00
Contributor
Copy link
@alwayscoding wrote in #146 (comment):
* (Optional) check if there is a review in progress * Create a PR: Store a file on disk, forgejo-cli does have a helper for that * Comments etc * Finish review: send comments if any, or approve
Yeah, I had a similar workflow in mind. One subcommand I would add to the workflow is a check-status subcommand. This one would simply verify, that the subject of the review isn't already outdated. In case it is we could then inform the user about that, prompt them to open a new review and delete the current one
Nvm, the status subcommand above would cover that. So fj pr review [id] status would check that the current review isn't outdated and, if it is, inform the user about that, prompting them to open a new review and delete the current one. Possibly we could let the user decide whether they want to open a new review in-place or to only delete the current review
@alwayscoding wrote in https://codeberg.org/forgejo-contrib/forgejo-cli/issues/146#issuecomment-17828204: > * (Optional) check if there is a review in progress > > * Create a PR: Store a file on disk, forgejo-cli does have a helper for that > > * Comments etc > > * Finish review: send comments if any, or approve Yeah, I had a similar workflow in mind. One subcommand I would add to the workflow is a Nvm, the check-status subcommand. This one would simply verify, that the subject of the review isn't already outdated. In case it is we could then inform the user about that, prompt them to open a new review and delete the current onestatus subcommand above would cover that. So fj pr review [id] status would check that the current review isn't outdated and, if it is, inform the user about that, prompting them to open a new review and delete the current one. Possibly we could let the user decide whether they want to open a new review in-place or to only delete the current review
alwayscoding commented 2026-06-24 15:24:01 +02:00
Copy link
Ok sounds good! How would you feel about starting a branch and trying out fj pr review [id]? This would be needed to initially check whether the PR exists 🙂
Ok sounds good! How would you feel about starting a branch and trying out fj pr review [id]? This would be needed to initially check whether the PR exists 🙂
Fluffinity commented 2026-06-25 16:19:39 +02:00
Contributor
Copy link
Do you mean starting a branch on my repo with the initial skeleton for a The heat is melting my brain a bit ^^'fj pr review [id] command? Just want to make sure I understand you correctly.
Ok, I realized only now that we already have these commands in there. That's on me for not using the binary compiled from main but using the latest release. With that out of the way, I do have an open PR to test things with on my forgejo-cli fork with a currently active review (ID 1)
Do you mean starting a branch on my repo with the initial skeleton for a The heat is melting my brain a bit ^^' Ok, I realized only now that we already have these commands in there. That's on me for not using the binary compiled from main but using the latest release. With that out of the way, I do have an open PR to test things with on my forgejo-cli fork with a currently active review (ID 1)fj pr review [id] command? Just want to make sure I understand you correctly.
Fluffinity commented 2026-06-25 20:51:43 +02:00
Contributor
Copy link
@Cyborus for clarification: did you imagine the pr review status command for use on the the review that is currently drafted or to check on the status of an existing review?
@Cyborus for clarification: did you imagine the pr review status command for use on the the review that is currently drafted or to check on the status of an existing review?
Cyborus commented 2026-06-26 01:47:22 +02:00
Author
Member
Copy link
It's been so long since I posted that that I don't remember. Don't feel like you need to follow that design too strictly, you'll likely learn how it can be improved as you work on it.
It's been so long since I posted that that I don't remember. Don't feel like you need to follow that design too strictly, you'll likely learn how it can be improved as you work on it.
alwayscoding commented 2026-06-26 04:57:14 +02:00
Copy link
@Fluffinity All good! Happy to offer guidance if you need it 🙂 Can comment in your PR branch if that's preferred
@Fluffinity All good! Happy to offer guidance if you need it 🙂 Can comment in your PR branch if that's preferred
Fluffinity commented 2026-06-26 12:17:08 +02:00
Contributor
Copy link
@Fluffinity All good! Happy to offer guidance if you need it 🙂 Can comment in your PR branch if that's preferred
It's fine to keep this here. I'd like this issue to exist for working out a fitting workflow, including the commands and how they should function. By the time, this stuff has been ironed out, the code is hopefully already ready for a PR.
> @Fluffinity All good! Happy to offer guidance if you need it :slightly_smiling_face: Can comment in your PR branch if that's preferred It's fine to keep this here. I'd like this issue to exist for working out a fitting workflow, including the commands and how they should function. By the time, this stuff has been ironed out, the code is hopefully already ready for a PR.
👍 1
Sign in to join this conversation.
No Branch/Tag specified
main
renovate/lock-file-maintenance
renovate/serde-saphyr-0.x
docs/contributing
no-port-in-refspec
login/v16.next.forgejo.org
crates-io-publish
0.5.x
compile-time-fluent
0.4.x
localization-alias-demo
api_url_field
v0.5.0
v0.4.1
v0.4.0
v0.3.0
v0.2.0
v0.1.1
v0.1.0
v0.0.4
v0.0.3
v0.0.2
v0.0.1
Labels
Clear labels[ Kind/Breaking Breaking change that won't be backward compatible
](#)[ Kind/Bug Something is not working
](#)[ Kind/Design Discussion about UI/UX design
](#)[ Kind/Documentation Documentation changes
](#)[ Kind/Enhancement Improve existing functionality
](#)[ Kind/Feature New functionality
](#)[ Kind/Security This is security issue
](#)[ Kind/Testing Issue or pull request related to testing
](#)[ Kind/Upstream This is an issue with upstream software (Forgejo) that is probably not our fault
](#)
[ Priority
Critical The priority is critical
](#)[ Priority
High The priority is high
](#)[ Priority
Low The priority is low
](#)[ Priority
Medium The priority is medium
](#)
[ Reviewed
Confirmed Issue has been confirmed
](#)[ Reviewed
Duplicate This issue or pull request already exists
](#)[ Reviewed
Invalid Invalid issue
](#)[ Reviewed
Won't Fix This issue won't be fixed
](#)
[ Status
Abandoned Somebody has started to work on this but abandoned work
](#)[ Status
Blocked Something is blocking this issue or pull request
](#)[ Status
Need More Info Feedback is required to reproduce issue or to continue work
](#)
[ Suspicious
](#)
No labels Kind/Breaking Kind/Bug Kind/Design Kind/Documentation Kind/Enhancement Kind/Feature Kind/Security Kind/Testing Kind/Upstream [ Priority
Critical ](/forgejo-contrib/forgejo-cli/issues?labels=173012) [ Priority
High ](/forgejo-contrib/forgejo-cli/issues?labels=173013) [ Priority
Low ](/forgejo-contrib/forgejo-cli/issues?labels=173015) [ Priority
Medium ](/forgejo-contrib/forgejo-cli/issues?labels=173014) [ Reviewed
Confirmed ](/forgejo-contrib/forgejo-cli/issues?labels=173007) [ Reviewed
Duplicate ](/forgejo-contrib/forgejo-cli/issues?labels=173005) [ Reviewed
Invalid ](/forgejo-contrib/forgejo-cli/issues?labels=173006) [ Reviewed
Won't Fix ](/forgejo-contrib/forgejo-cli/issues?labels=173008) [ Status
Abandoned ](/forgejo-contrib/forgejo-cli/issues?labels=173011) [ Status
Blocked ](/forgejo-contrib/forgejo-cli/issues?labels=173010) [ Status
Need More Info ](/forgejo-contrib/forgejo-cli/issues?labels=173009) Suspicious
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
Notifications Subscribe
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".
No due date set.
Dependencies
No dependencies set.
Reference
forgejo-contrib/forgejo-cli#146
WritePreview
Loading…
Add table
| Rows | | | Columns | |
CancelOK
Add a link
Url Description Hint: With a URL in your clipboard, you can paste directly into the editor to create a link.
CancelOK
CancelSave
Reference in a new issue
Repository
forgejo-contrib/forgejo-cli
Title
Body
Create issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
No Yes