-
Notifications
You must be signed in to change notification settings - Fork 5
fix: enhance protolint_flags option #4
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
fix: enhance protolint_flags option #4
Conversation
* Resolved argument handling in protolint_flags option * Uses .protolint.yaml if present * Updated README.md Closes yoheimuta#3
I tested it locally with act and it works as expected. |
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.
Thank you for your contribution!
You're correct that ${INPUT_PROTOLINT_FLAGS} shouldn't have been enclosed in double quotations.
And I've posted a minor question.
README.md
Outdated
@@ -8,6 +8,7 @@ | |||
[](https://github.com/haya14busa/action-bumpr) | |||
|
|||
This GitHub Action runs [protolint](https://github.com/yoheimuta/protolint) with [reviewdog](https://github.com/reviewdog/reviewdog). | |||
By default, action-protolint uses the `.protolint.yaml` configuration file if it exists. |
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 assume that protolint would work in that manner, so action-protolint shouldn't need to specify the .protolint.yaml
file located in the current working directory. Have you confirmed whether protolint failed to locate that file?
protolint will automatically search a current working directory for the config file by default and successive parent directories all the way up to the root directory of the filesystem. And it can search the specified directory with -config_dir_path flag. It can also search the specified file with --config_path flag.
https://github.com/yoheimuta/protolint#configuring
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.
It looks like the problem might be related to the quotes around ${INPUT_PROTOLINT_FLAGS}
. I removed the code to check the existence of the .protolint.yaml
config file, protolint can still find the config file.
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.
LTGM!
🚀 [bumpr] Bumped! New version:v1.1.0 Changes:v1.0.0...v1.1.0 |
Closes #3