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

feature: add directory argument #15

Merged
merged 14 commits into from
May 16, 2020
Merged

Conversation

hazcod
Copy link
Contributor

@hazcod hazcod commented Nov 25, 2019

Makes it possible to run the action in a subdirectory.

@hazcod
Copy link
Contributor Author

hazcod commented Nov 26, 2019

CC @mattn @haya14busa

Copy link
Member

@haya14busa haya14busa left a comment

Choose a reason for hiding this comment

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

IIRC, github-pr-check doesn't work well with subdirectory run.
Can you confirm it works or consider to use golangci_lint_flags to specify sub dir?

Maybe we can improve the document too.

README.md Outdated
@@ -87,6 +90,7 @@ jobs:
# Can pass --config flag to change golangci-lint behavior and target
# directory.
golangci_lint_flags: "--config=.github/.golangci.yml ./testdata"
directory: subdirectory/
Copy link
Member

Choose a reason for hiding this comment

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

Can you specify sub dir like this with golangci_lint_flags?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aslafy-z
Copy link
Contributor

aslafy-z commented Jan 3, 2020

Related issues:
golangci/golangci-lint#828
golangci/golangci-lint#855
golangci/golangci-lint#865
golangci/golangci-lint#870

Is there any way we could add this feature without breaking the github-[pr-]check reporters? We could maybe pass down the sub-path to reviewdog and let him add it to the matched paths?

@aslafy-z
Copy link
Contributor

aslafy-z commented Jan 4, 2020

@hazcod
Copy link
Contributor Author

hazcod commented Jan 7, 2020

ping @haya14busa

@haya14busa
Copy link
Member

github-pr-check doesn't work well with subdirectory run.

As I said, github-pr-check won't work with subdir, so I cannot merge it at this point.

bwt, is it really reproducible with latest reviewdog/golangci-lint binary?

I cloned ironPeakServices/iron-redis and ran following command. It succeeded.

$  golangci-lint run --enable-all ./...   

@hazcod
Copy link
Contributor Author

hazcod commented Jan 7, 2020

@haya14busa seems not with reviewdog/action-golangci-lint@v1.1.3: https://github.com/ironPeakServices/iron-redis/pull/15/checks?check_run_id=377848053
level=error msg="Running error: context loading failed: package github.com/go-redis/redis: no go files to analyze"

@haya14busa
Copy link
Member

Can you try v1.1.6 (or v1)?

@hazcod
Copy link
Contributor Author

hazcod commented Jan 7, 2020

@evanfuller
Copy link

any update on whether this will merge?

@samsondav
Copy link

We are also seeing this problem.

Copy link
Member

@haya14busa haya14busa left a comment

Choose a reason for hiding this comment

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

reviewdog can now support sub-dir run for every reporter.
https://github.com/reviewdog/reviewdog/blob/master/CHANGELOG.md#v0100---2020-05-07

LGTM other than nit comment.

action.yml Outdated Show resolved Hide resolved
action.yml Outdated Show resolved Hide resolved
@haya14busa haya14busa merged commit af8c28e into reviewdog:master May 16, 2020
@github-actions
Copy link
Contributor

🚀 [bumpr] Bumped! New version:v1.6.0 Changes:v1.5.0...v1.6.0

@review-dog
Copy link
Member

Hi, @hazcod! We merged your PR to reviewdog! 🐶
Thank you for your contribution! ✨

We just invited you to join the @reviewdog organization on GitHub.
Accept the invite by visiting https://github.com/orgs/reviewdog/invitation.
By joining the team, you'll be a part of reviewdog community and can help the maintainance of reviewdog.

Thanks again!

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.

None yet

6 participants