Repository navigation
Add lock and unlock commands for issues and pull requests - #5333
Merged
Merged
Conversation
Lock will lock and unlock both issues and pull requests
chemotaxis
force-pushed
the
gh-issue-lock
branch
from
March 30, 2022 07:10
be544c8 to
d1aa284
Compare
As originally designed in the issue discussion, a single function `NewCmdLock()` with a parameter to lock or unlock was proposed. However, after playing around with a couple different designs, it seems best to create two separate public functions and one private function to do the common work. Using two public functions seems to make sense because the api for locking is different from the api for unlocking. Therefore, the documentation for both are different and keeping them in separate functions would make it easier to maintain the documentation.
- Changed function to method
- Moved additional common options to method
- Remove redundant documentation
- Cobra sets documentation in the Command struct.
These are needed to know if an issue is an actual issue or a pull request.
Add queries for "ActiveLockReason" and "Locked"
chemotaxis
force-pushed
the
gh-issue-lock
branch
2 times, most recently
from
April 11, 2022 08:50
b88ffa2 to
c1f98dc
Compare
- Fix error if found an issue while using `gh pr lock/unlock` or vice versa
- Added additional types
- Used githubv4 types
- Added "relock" state
- If the conversation is already locked you have two choices: try to
lock it again or do nothing. Do nothing is easy. But, if you
want to change the lock reason, you need to first unlock the
conversation and then lock it again.
- Added survey to confirm if you want to relock
- Added formatted print statements
- Switch to underscores - Revise error message
Rather than saving the intended lock state and calling a method depending on the lock state, just call the method directly. By the time you need to the padlock state, you already know which method to use; no need to first change the lock state than call the method. Also, refactored print/error messages that are conditional.
I noticed that PadlockState didn't really have anything to do with the LockOptions and it was easy to call an incorrect locking function that didn't match the PadlockState. Now, you pass in the state as an argument and you simply call the appropriate function instead of setting PadlockState and then calling the correct function. - Other touch ups and refactoring
Use switch statements for regular branching code.
As much as I like keeping statements as flat as possible, this not-as-flat revision just seems easier to read.
chemotaxis
force-pushed
the
gh-issue-lock
branch
from
April 29, 2022 04:34
3f5ba37 to
718bb80
Compare
Contributor
Author
|
I apologize if the history on this pull request is a little messy, but I went through several drafts before arriving at this current version. I haven't written any tests yet, but I wanted to see if the overall behavior was acceptable. Some notes: There seem to be several methods with overlapping functionality in It was also somewhat tricky to address two separate functions (lock/unlock) and two separate types (issues/pull requests) simultaneously. Hopefully my current solution addresses everything fairly cleanly. Finally, since the current version of |
chemotaxis
marked this pull request as ready for review
April 29, 2022 05:35
cliAutomation
added
the
external
Update a somewhat stale feature branch with the latest stable commits.
vilmibm
self-requested a review
August 22, 2022 16:16
vilmibm
suggested changes
Aug 22, 2022
vilmibm
left a comment
vilmibm
left a comment
Contributor
There was a problem hiding this comment.
This is a great start. I'm cool with the UX and appreciate the effort put into making this work across issues and prs.
Add recent changes, especially changes to `api`.
Similar to commit 45f1a71
While I usually like explicitly setting all values, getting rid of setting the empty string streamlines map construction and testing. The reasonsMap will return the nil value because the empty string is not in the map.
Make sure reasons were added in the correct order.
Resolved conflicts mainly around pinning comments.
Contributor
|
I've:
|
vilmibm
enabled auto-merge
December 20, 2022 23:50
vilmibm
self-requested a review
December 20, 2022 23:53
vilmibm
approved these changes
Dec 21, 2022
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Fixes #5020.
This pull request adds
lockandunlockto issues and pull requests.unlockwill unlock a conversation if it was previously locked. Otherwise, it will do nothing.lockwill lock a conversation if it was previously unlocked. You can optionally specify a reason for locking from a set of fixed reasons.If the issue or pull request is already locked,
ghwill prompt you to confirm if you want to "relock" a conversation or abort the "relock" if you forgot or didn't know the conversation was already locked. The primary use for relocking will probably be to change the lock reason.I tried just sending a lock request again with an updated reason, but nothing changes. You first have to unlock the conversation, then lock it with the updated reason.
The error messages are parameterized on several variables including the parent command (
issueorpr), what the intended action was (lock or unlock), the reason for locking, etc. For example, ifissue #1exists, but we used the commandgh pr lock 1, the error message will say thatissue #1exists, but notpull request #1. It then suggests to usegh issue lock 1instead.