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

Add working directory flag to allow use of action from non-root directory #19

Merged
merged 7 commits into from
May 8, 2020
Merged

Add working directory flag to allow use of action from non-root directory #19

merged 7 commits into from
May 8, 2020

Conversation

LaurenceGA
Copy link

@LaurenceGA LaurenceGA commented Feb 3, 2020

Fixes #18

This action assumes that your package.json is in the root directory of your repository (or I guess that you have eslint globally installed). I think it's common to have eslint in your package.lock in a subdirectory. Hence, it's necessary to tell the action not only where to run eslint, but also where to find it. Unless I'm missing something, I'm pretty sure this is required for my use case anyway.

eslint_flags is fine for specifying what to run eslint on, but the action still looks for eslint in the root directory, so it's insufficient for this use case.

Please let me know if I missed anything!

  • Add test

@LaurenceGA LaurenceGA requested a review from haya14busa February 3, 2020 06:54
@aslafy-z
Copy link

aslafy-z commented Feb 3, 2020

Reporting to Reviewdog will only match paths if eslint reports absolutes ones. I'm not able to test right now, is it the case?

@haya14busa
Copy link
Member

Same as reviewdog/action-golangci-lint#15, github-pr-check and github-check reporter doesn't support sub-directory run currently. reviewdog/reviewdog#461

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.

action.yml Outdated Show resolved Hide resolved
@LaurenceGA
Copy link
Author

Reporting to Reviewdog will only match paths if eslint reports absolutes ones. I'm not able to test right now, is it the case?

It seems to me that eslint always reports absolute URLs anyway? So I assume there are other reasons that reviewdog can't be run from a subdirectory than the linter's output URLs 🤔

@LaurenceGA
Copy link
Author

Same as reviewdog/action-golangci-lint#15, github-pr-check and github-check reporter doesn't support sub-directory run currently. reviewdog/reviewdog#461

I've updated to try to allow for running eslint (and reviewdog) from the root directory, but on the given workdir.

It seems to have mostly worked. The check failed (as expected, I suppose) and the one with reporter=github-pr-review passed, but I guess it would have failed the actual review.

It was super easy to get npm to do its work in a subdirectory when run from root, but eslint was a bit trickier.

A few things worth noting:
Even if you specify workdir to be a sub folder, eslint's flags' file references are still relative to the root directory. So I made it that by default it passes "workdir" instead of ".", but this could be confusing if you try to override the flags.
So if you want to specify a subfolder of your subproject, you still have specify "subproject/subfolder/" instead of just "subfolder/", even if you have already specified workdir. So I doesn't entirely function like you'd expect a working directory to 😞
Also when I try running eslint on the subprojects' root directory from the main project's root directory, it seems to fail to ignore node_modules. Not sure why that is...

These may be reasons to just hold off until reviewdog supports running in subdirectories...

entrypoint.sh Outdated Show resolved Hide resolved
@LaurenceGA LaurenceGA requested a review from haya14busa May 8, 2020 02:18
@LaurenceGA
Copy link
Author

📝 Tests don't actually run properly as the PR is from my forked repo 🤔

@ChadTaljaardt
Copy link

Thanks for the fast response to the requested changed @LaurenceGA

@haya14busa
Copy link
Member

The test worked with minor modifications. #25
As it's actually changing the workdir, we no longer need to prepend subdir in eslint_flags as you wrote (#19 (comment))

haya14busa added a commit that referenced this pull request May 8, 2020
Add working directory flag to allow use of action from non-root directory  (Follow up of #19)
@haya14busa haya14busa merged commit 3bd1467 into reviewdog:master May 8, 2020
@haya14busa
Copy link
Member

I merged it and released a new version.
I also changed the flag name back to workdir as I prefer workdir because I think it's more descriptive than just a directory.

Thank you for your contribution, @LaurenceGA !

@LaurenceGA LaurenceGA deleted the working-dir branch May 8, 2020 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How do you use this action in a folder not in the root directory?
4 participants