-
-
Notifications
You must be signed in to change notification settings - Fork 8k
Use GH action to install reviewdog #27619
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
Conversation
I guess my personal opinion is that if we are going to curl | sh
, I'd rather be up front about it and not hide it in layers of obfuscation... (That is, in fact, what reviewdog's action does, modulo allowing wget instead of curl and the precise location that it gets installed to)
It just feels like we are adding steps and not getting any benefit from it, and just making it harder to figure out what went wrong if it does go wrong (though the counterargument there is that we would have other people looking for the error simultaneously).
It does look like there was a temporary problem affecting some PRs where the install script required an argument but that appears to have been reverted such that no argument is equivalent to "latest"
I was thinking if they change the URL it's one less thing to maintain on our end, and I think having other eyes on it so bugs get reported and fixed faster outweights us having to track down something by ourselves when it goes wrong. There are reportedly 1.9k repos using the action, so it is widely used.
19083e7
to
680b70b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in favor of deferring this to an action.
PR summary
I presume this is more recent than when reviewdog was setup. Saves a few lines and makes for easier maintenance in the future.
PR checklist