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.

Sign up for GitHub

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

Jump to bottom

chore(deps): remove urfave/cli #2866

Merged
merged 1 commit into from
Dec 23, 2022
Merged

chore(deps): remove urfave/cli #2866

merged 1 commit into from
Dec 23, 2022
+1 −12

Conversation

krrrr38
Copy link
Contributor

@krrrr38 krrrr38 commented Dec 23, 2022 •
edited by nitrocode
Loading

what

remove urfave/cli dependency

why

I tried to upgrade urfave/cli for #2858, but not needed by following investigation.

This library is introduced for cli.NewApp, but currently atlantis use cobra, so cli.NewExitError has no meanings

So cli.NewExitError has no meaning.

references

@krrrr38 krrrr38 requested a review from a team as a code owner December 23, 2022 09:00
krrrr38
krrrr38 commented Dec 23, 2022
View reviewed changes
server/server.go
@@ -958,7 +957,7 @@ func (s *Server) Start() error {

ctx, _ := context.WithTimeout(context.Background(), 5*time.Second) // nolint: vet
if err := server.Shutdown(ctx); err != nil {
return cli.NewExitError(fmt.Sprintf("while shutting down: %s", err), 1)
return fmt.Errorf("while shutting down: %s", err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

atlantis use cobra, so cli.ExitCoder is handled as simple err.

https://github.com/urfave/cli/blob/8c0fb2dba4f52d1a93101ee529a490da1dcb6718/errors.go#L77-L96

nitrocode
nitrocode approved these changes Dec 23, 2022
View reviewed changes
Copy link
Member

@nitrocode nitrocode left a comment

Choose a reason for hiding this comment

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

Nice work! I wish there was a tool to see which dependencies are barely used such as the one you found.

The fewer dependencies we have to manage, the faster our builds, the less maintenance we have to do, and the more resilient the app becomes.

@nitrocode nitrocode changed the title remove urfave/cli chore(deps): remove urfave/cli Dec 23, 2022
@nitrocode nitrocode merged commit b3d0847 into runatlantis:main Dec 23, 2022
@nitrocode nitrocode added this to the 0.22.0 milestone Dec 23, 2022
@albertorm95 albertorm95 mentioned this pull request May 19, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nitrocode nitrocode nitrocode approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.22.0
Development

Successfully merging this pull request may close these issues.
2 participants
@krrrr38 @nitrocode