Skip to content
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

final isolation repo cleanup #1435

Merged
merged 2 commits into from
Jul 28, 2020
Merged

final isolation repo cleanup #1435

merged 2 commits into from
Jul 28, 2020

Conversation

vilmibm
Copy link
Contributor

@vilmibm vilmibm commented Jul 27, 2020

Closes #1405
Part of #1082

This PR:

  • isolates the repo credits easter egg
  • removes command/repo.go in favor of pkg/cmd/repo/repo.go

I could have gone farther and re-exported the various NewCmd* functions from the new repo package
but it didn't seem worth it right now.

@vilmibm vilmibm added this to Backlog 🗒 in The GitHub CLI via automation Jul 27, 2020
@vilmibm vilmibm requested a review from mislav July 27, 2020 19:10
@vilmibm vilmibm moved this from Backlog 🗒 to Needs review 🤔 in The GitHub CLI Jul 27, 2020
Copy link
Contributor

@mislav mislav left a comment

Choose a reason for hiding this comment

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

Looks good!

I wonder if the Command in pkg/cmd/repo/repo.go should nest other repo commands underneath it, or should it remain the responsibility of root.go to properly create a command tree? Right now it does not matter either way, so let's leave it as-is for now and revisit later based on how we feel.

The GitHub CLI automation moved this from Needs review 🤔 to Needs to be merged 🎉 Jul 28, 2020
@vilmibm
Copy link
Contributor Author

vilmibm commented Jul 28, 2020

I couldn't decide either way and figured we could punt on it as you suggest. I kind of like every AddCommand being in one place but it does make for a very long root.go.

@vilmibm vilmibm merged commit f486cc4 into trunk Jul 28, 2020
The GitHub CLI automation moved this from Needs to be merged 🎉 to Pending Release 🥚 Jul 28, 2020
@mislav mislav deleted the repo-cleanup branch August 3, 2020 08:01
@mislav mislav moved this from Pending Release 🥚 to Done 💤 in The GitHub CLI Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
The GitHub CLI
  
Done 💤
Development

Successfully merging this pull request may close these issues.

isolate repo commands
2 participants