Skip to content

Add gh ssh-key delete <id>#6273

Merged
samcoe merged 8 commits into
cli:trunkfrom
nsmag:ssh-key-delete
Oct 12, 2022
Merged

Add gh ssh-key delete <id>#6273
samcoe merged 8 commits into
cli:trunkfrom
nsmag:ssh-key-delete

Conversation

@nsmag
Copy link
Copy Markdown
Contributor

@nsmag nsmag commented Sep 15, 2022 •
edited
Loading

Resolves #6136.

Add an ability to delete an SSH key gh ssh-key delete <id>.

$ gh ssh-key delete 70929824
? Type Test Key to confirm deletion: Test Key
✓ SSH key "Test Key" (70929824) deleted from your account

@nsmag nsmag requested a review from a team as a code owner September 15, 2022 14:23
@nsmag nsmag requested review from vilmibm and removed request for a team September 15, 2022 14:23
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label Sep 15, 2022
@nsmag nsmag changed the title Add gh ssh-key delete {id} Add gh ssh-key delete <id> Sep 19, 2022
@samcoe samcoe self-assigned this Sep 19, 2022
@samcoe samcoe self-requested a review September 19, 2022 09:42
samcoe
samcoe suggested changes Sep 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@samcoe samcoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nsmag Thanks for starting on this feature. It is coming along nicely. I left some comments requesting changes. Let me know if you have any questions about them.

Thanks for being thorough and adding tests!

Comment thread pkg/cmd/ssh-key/delete/delete.go Outdated
Comment thread pkg/cmd/ssh-key/delete/delete.go Outdated
Comment thread pkg/cmd/ssh-key/delete/delete.go Outdated
@nsmag
Copy link
Copy Markdown
Contributor Author

nsmag commented Sep 20, 2022

@samcoe Fixed as requested. About the prompt, I try to replicate label delete behavior as you suggested but I think I cannot do the "prompt again" with the Prompter interface.

@nsmag nsmag requested review from samcoe and removed request for vilmibm September 20, 2022 11:40
@samcoe
Copy link
Copy Markdown
Contributor

samcoe commented Sep 22, 2022

@nsmag That is a good call out. #6320 is going to get merged shortly, so if you don't mind waiting a bit after it is merged we can use ConfirmDeletion and have this new command be consistent with the other commands that have the same workflow.

@nsmag
Copy link
Copy Markdown
Contributor Author

nsmag commented Sep 22, 2022

@samcoe Sure. That'll come in handy.

@nsmag nsmag mentioned this pull request Sep 23, 2022
Closed
4 tasks
@vilmibm
Copy link
Copy Markdown
Contributor

vilmibm commented Oct 4, 2022

#6320 just got merged.

@nsmag
Copy link
Copy Markdown
Contributor Author

nsmag commented Oct 5, 2022

@samcoe Implemented ConfirmDeletion into this.

Also the corresponding gpg-key delete is done #6360.

samcoe
samcoe approved these changes Oct 12, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@samcoe samcoe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nsmag This looks great, I think it is ready to 🚢

@samcoe samcoe enabled auto-merge (squash) October 12, 2022 08:15
@samcoe samcoe merged commit 5962bc4 into cli:trunk Oct 12, 2022
@nsmag nsmag deleted the ssh-key-delete branch October 14, 2022 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@samcoe samcoe samcoe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@samcoe samcoe

Labels

external pull request originating outside of the CLI core team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add "remove" feature to ssh-key

4 participants

@nsmag @samcoe @vilmibm @cliAutomation