Skip to content

Conversation

arvgord
Copy link
Contributor

@arvgord arvgord commented Oct 28, 2023

  • Resolved argument handling in protolint_flags option
  • Uses .protolint.yaml if present
  • Updated README.md

Closes #3

* Resolved argument handling in protolint_flags option
* Uses .protolint.yaml if present
* Updated README.md

Closes yoheimuta#3
@arvgord
Copy link
Contributor Author

arvgord commented Oct 28, 2023

I tested it locally with act and it works as expected.
Снимок экрана от 2023-10-29 02-09-23
In the logs I see that all the arguments are separated. And the .protolint.yaml file is used if it exists.
изображение

Copy link
Owner

@yoheimuta yoheimuta left a 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 @@
[![action-bumpr supported](https://img.shields.io/badge/bumpr-supported-ff69b4?logo=github&link=https://github.com/haya14busa/action-bumpr)](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.
Copy link
Owner

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

Copy link
Contributor Author

@arvgord arvgord Oct 29, 2023

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.

Copy link
Owner

@yoheimuta yoheimuta left a comment

Choose a reason for hiding this comment

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

LTGM!

@yoheimuta yoheimuta merged commit a7c658b into yoheimuta:master Oct 30, 2023
@github-actions
Copy link

🚀 [bumpr] Bumped! New version:v1.1.0 Changes:v1.0.0...v1.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to specify configuration file path
2 participants