-
Notifications
You must be signed in to change notification settings - Fork 84
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
Configure NODE_PATH to load plugin from GitHub workspace #19
Conversation
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 like this change. It's a reasonable solution to #16. I want some docs in the README/recipes
section on how to use this. As far as I understand it, we want 2 situations to be documented, right?
- I am developing a new plugin, and want to test it with this action from the repo in which I am developing that new plugin.
- I am developing some web project, and want to run this action with a plugin that I can get from
npm
.
src/index.js
Outdated
@@ -1,8 +1,21 @@ | |||
// Append the node_modules of the github workspace and the node_modules of this action | |||
// to NODE_PATH. This supports lighthouse plugins - all the workspace needs to do it |
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 think this comment is enough, then explain more in detail in the docs.
// to NODE_PATH. This supports lighthouse plugins - all the workspace needs to do it | |
// to NODE_PATH. |
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 the elegant fix for plugins support 👍
Minor comments regarding node_modules
folder:
- I like the omission of the
.yarn-integrity
.DS_Store
files should be deleted as well. I think, via .gitignore,.yarnclean
uses only on install.
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.
LGTM!
Fixes #16
This change supports plugins w/ no changes to a workspace config, other than running
npm/yarn install
. See my example repo: https://github.com/connorjclark/lighthouse-plugin-sample-project/blob/master/.github/workflows/main.yml