Rename local repo by gh repo rename
#9940
Conversation
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
A good start 🙌 I've left a few comments on the code and have a few other thoughts:
- I think we should also include this in the prompter, i.e. "Would you also like to rename the repo directory to ?".
- I think we should check include something in the confirmation prompt about also renaming the repo directory if the flag is included. Right now it only confirms for the repo rename.
- We should write some tests for this.
I'm having some thoughts about putting the directory renaming logic in here, too, but they haven't fully formed yet... I think the culmination of those might depend on what the tests reveal for the renameRepoDir() function. I sort of suspect that we'll need some sort of bespoke behavior for Windows vs Mac/Linux, and that hints at an interface to me. No point abstracting it until we discover it, though 🙂
pkg/cmd/repo/rename/rename.go
Outdated
| @@ -170,3 +180,16 @@ func updateRemote(repo ghrepo.Interface, renamed ghrepo.Interface, opts *RenameO | |||
|
|
|||
| return remote, err | |||
| } | |||
|
|
|||
| func renameRepoDir(newRepoName string, opts *RenameOptions) (err error) { | |||
There was a problem hiding this comment.
I'd love to see some tests for this. I wonder if we can cover the different os use-cases in those tests 🤔
There was a problem hiding this comment.
Should these tests need to be done now? Since renaming the local folder isn't working yet, I think we can write the tests after the feature is implemented
There was a problem hiding this comment.
Ah, that's up to you. I'm not trying to prescribe the order in which you implement the solution for this, but instead highlighting that we'll want test coverage before this ships.
FWIW, though, I think writing the tests might inform how best to finish the feature, though that's my general workflow as is. I tend to lean on TDD when I'm trying to implement something with some unknown unknowns, like how different os's will handle this, though I know that's not how many folks like to work 🙂
Message of `-N` flag Co-authored-by: Tyler McGoffin <[email protected]>
Co-authored-by: Tyler McGoffin <[email protected]>
pkg/cmd/repo/rename/rename.go
Outdated
| if opts.DoConfirm && !opts.RenameLocalDir { | ||
| var confirmed bool | ||
| if confirmed, err = opts.Prompter.Confirm("Would you also like to rename the repo directory to ?", false); err != nil { | ||
| return err | ||
| } | ||
| if confirmed { | ||
| opts.RenameLocalDir = true | ||
| } | ||
| } | ||
| if opts.RenameLocalDir { | ||
| if err = renameRepoDir(newRepoName, opts); err != nil { | ||
| fmt.Fprintf(opts.IO.ErrOut, "Failed to rename local directory: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
🤔 interesting idea putting this all in its own place. What's the UX look like for this? I was imagining something like:
# When the user doesn't specify anything
$ gh repo rename
? Rename <owner/repo> to: <new-name>
? Would you also like to rename the local directory to the same name? (y/N) y
✓ Renamed repository owner/<new-name> and local directory
✓ Updated the "origin" remote
# When the user specifies the repo name
$ gh repo rename <new-name>
? Rename <owner/repo> to <new-name>? (y/N) y
✓ Renamed repository owner/<new-name>
✓ Updated the "origin" remote
# When the user includes the --rename-dir flag
$ gh repo rename --rename-dir
? Rename <owner/repo> and local directory to: <new-name>
✓ Renamed repository owner/<new-name> and local directory
✓ Updated the "origin" remote
# When the user specifies the repo name and includes the --rename-dir flag
$ gh repo rename <new-name> --rename-dir
? Rename <owner/repo> to <new-name> and the local directory name? (y/N) y
✓ Renamed repository owner/<new-name> and local directory
✓ Updated the "origin" remote
# When the user specifies the repo name and includes the --rename-dir and --yes flags
$ gh repo rename <new-name> --rename-dir --yes
✓ Renamed repository owner/<new-name> and local directory
✓ Updated the "origin" remoteI'll add this UX to the issue 👍
There was a problem hiding this comment.
@jtmcg thanks for the clear UI, checkout the latest commit!
pkg/cmd/repo/rename/rename.go
Outdated
| opts.RenameLocalDir = true | ||
| } | ||
| } | ||
| if opts.RenameLocalDir { |
There was a problem hiding this comment.
We should probably check somewhere that the name the user has chosen isn't already in-use. We should probably do this before the API call, even, and can error if it is.
|
|
||
| if !opts.RenameLocalDir { | ||
| var confirmed bool | ||
| if confirmed, err = opts.Prompter.Confirm("Would you also like to rename the local directory to the same name?", false); err != nil { |
There was a problem hiding this comment.
I haven't been able to get this prompt when running the command gh repo rename
There was a problem hiding this comment.
Sorry for the late reply😥. I've just test gh repo rename again, the prompt does show up.
| if renamedLocalDir { | ||
| fmt.Fprintf(opts.IO.Out, "%s Renamed repository %s and local directory\n", cs.SuccessIcon(), ghrepo.FullName(newRepo)) | ||
| } else { | ||
| fmt.Fprintf(opts.IO.Out, "%s Renamed repository %s\n", cs.SuccessIcon(), ghrepo.FullName(newRepo)) | ||
| } |
There was a problem hiding this comment.
There's some weird behavior in how the local dir interacts with the rename on mac, here:
# In the repo you are going to rename
$ pwd
<path>/original-repo-name
$ gh repo rename renamed-repo --rename-dir --yes
✓ Renamed repository <path>/renamed-repo and local directory
✓ Updated the "origin" remote
$ pwd
<path>/original-repo-name
$ ls <path>
renamed-repoIn other words, after the rename, the old directory still exists as long as we are in it. I think it may be helpful to do some navigating through the os so we end up in the target directory after the rename.
There was a problem hiding this comment.
My message is different from yours (I’m using Windows)😱. I have an additional error message about failing to rename:
$ gh repo rename renamed-repo --rename-dir --yes
Failed to rename local directory: rename C:/Users/user/Desktop/origin-repo C:\Users\user\Desktop\renamed-repo: The process cannot access the file because it is being used by another process.
✓ Renamed repository benebsiny/ltcd
✓ Updated the "origin" remote
In other words, after the rename, the old directory still exists as long as we are in it. I think it may be helpful to do some navigating through the os so we end up in the target directory after the rename.
yeah, this is exactly the challenge when trying to resolve this issue
ghost
mentioned this pull request
Fixes #9695
An idea to address this issue, but it might run into the problem of not being able to rename the folder. Like I mentioned here, the issue happens because the gh process's working directory is the folder you’re trying to rename, which causes the error: "the process cannot access the folder because it is being used by another process." Any ideas about this?