search

mainmatter / ember-asset-size-action

Comment with the diff for the asset sizes on Pull Request
MIT License
22 stars 15 forks source link

Remove `npm ci` from action? #2

Closed mydea closed 4 years ago

mydea commented 4 years ago

Hey,

that action looks really nice, and I'd like to use it in our apps. However, we use yarn, so this fails on npm ci.

It would be awesome if we could take the npm ci out of this action, and leave that to be done in the workflow? That way, it could be used in any environment. What do you think about this idea?

PS: I've used this as a base for https://github.com/mydea/ember-cli-code-coverage-action/, thank you very much for the inspiration :)

mansona commented 4 years ago

Hey @mydea, thanks for checking out the action 👍

It's not currently possible to remove npm ci from the action because your PR could be adding new ember addons which would in turn change the asset size 🤔 and it needs to change branch and then install the dependencies again.

What we can do instead of removing it is we can add some logic for npm or yarn detection. I'm sure I have some code that we're using in another project that would be perfect for this, let me see if I can't add it to the project this morning 😄

mansona commented 4 years ago

@mydea I got it working in the end, can you try it out and let me know if it's working for you?

mydea commented 4 years ago

Thanks! Seems to work now! :+1: