foo-software / lighthouse-check-action

GitHub Action for running @GoogleChromeLabs Lighthouse audits with all the bells and whistles 🔔 Multiple audits, Slack notifications, and more!
https://github.com/marketplace/actions/lighthouse-check
MIT License
478 stars 24 forks source link

node_modules #32

Closed TatisLois closed 4 years ago

TatisLois commented 4 years ago

Hey! I was just wondering if it was intentional to push up node_modules to source? I think just the package-lock.json should suffice?

adamhenson commented 4 years ago

@TatisLois - unfortunately, this is the default way of creating a GitHub Action. I don't like it either, but the alternative adds caveats that cost more than the value.

When we first created this GitHub Action, there wasn't an alternative. I'll be re-evaluating this soon, but it's been on my radar since the beginning so I think it's fair to close this. Feel free to comment further if I didn't answer your question.

adamhenson commented 4 years ago

As an alternative to checking in your node_modules directory you can use a tool called zeit/ncc to compile your code and modules into one file used for distribution.

You have to compile node_modules and the source into one massive dist file that is committed. To do so you have to run zeit/ncc tooling locally before every push. I think it could be fairly easily automated... just doesn't seem like a better solution 😢

adamhenson commented 4 years ago

And just to clarify this is a JavaScript GitHub Action. You can also create Docker GitHub Actions which wouldn't require checking in node_modules.

TatisLois commented 4 years ago

And just to clarify this is a JavaScript GitHub Action. You can also create Docker GitHub Actions which wouldn't require checking in node_modules.

Thanks for the input, that makes a lot of sense. Would you be open to converting it into Docker GitHub Actions? I have general CI and docker experience but never worked on github actions but would be willing to take it on.

adamhenson commented 4 years ago

Thanks for the offer. No, it's more maintainable for us as a JavaScript GitHub Action.

adamhenson commented 4 years ago

Definitely annoying and surprising that GitHub hasn't provided a better solution. Some related chatter:

IgnorantSapient commented 3 years ago

@adamhenson Why can't we just do an npm install after checkout inside the main.yml file, any downsides to it? I can wire it up for you if you want.

adamhenson commented 3 years ago

@123aswin123 - This repo provides a GitHub Action. It is not consuming a GitHub Action.

To create a JavaScript GitHub Action you need to create an action metadata file as documented here. In this project the action metadata file exists here.

As documented by GitHub, you must commit node_modules, with one alternative.

From your terminal, commit your action.yml, index.js, node_modules, package.json, package-lock.json, and README.md files. If you added a .gitignore file that lists node_modules, you'll need to remove that line to commit the node_modules directory

The alternative (which I don't think is the best option for us):

As an alternative to checking in your node_modules directory you can use a tool called @vercel/ncc to compile your code and modules into one file used for distribution.

This issue and these comments are irrelevant to this project. Please re-direct your comments to GitHub, as this a part of the GitHub Actions API.