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

perf: use a docker image with prebuild commitlint deps #66

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

simllll
Copy link

@simllll simllll commented Nov 17, 2020

heavily inspired by https://github.com/wagoid/commitlint-github-action I though we could do the same for this linter action.

The npm install step took more than 1 minute in my tests, now the whole action compeltes within 10 seconds 👍

This PR is not very "cleaned up", I still thought I open it .. and if it's just an inspiration to do the same yourself ;-)

Thanks for your work!

@ghost
Copy link

ghost commented Nov 17, 2020

Sider detected 3 warnings on analyzing the commit 2b33484.

We recommend fixing them as possible by updating the dependencies, configuring the analysis tool, configuring sider.yml, turning off unused tools, and so on.

If you have problems or questions still, feel free to ask us via chat. 💬


You can turn off this notification if you don't want to receive it from now on.

@JulienKode
Copy link
Owner

Hi @simllll, thanks for submitting this pull request

This look very promising, so by using a docker image this is faster than using a transpiled modules ?

On my custom GitHub runner I do not have docker. Should we have two version ?

  • one with docker
  • one without docker

Comment on lines +48 to +57
"@commitlint/config-angular": "^11.0.0",
"@commitlint/config-conventional": "^11.0.0",
"@commitlint/config-lerna-scopes": "^11.0.0",
"@commitlint/config-patternplate": "^11.0.0",
"@commitlint/format": "^11.0.0",
"@commitlint/lint": "^11.0.0",
"@commitlint/load": "^11.0.0",
"commitlint-config-jira": "^1.4.1",
"commitlint-plugin-jira-rules": "^1.4.0",
"conventional-changelog-lint-config-canonical": "^1.0.0",
Copy link
Owner

@JulienKode JulienKode Nov 21, 2020

Choose a reason for hiding this comment

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

Currently we have limited number of commit lint module because:

  • we load the config
  • we run the lint

We are just doing that, every people can use this library as they want. If they need another commit lint module they can just install the dependencies that they want at the version that they want in their own package.json

@ssbarnea
Copy link

ssbarnea commented Aug 3, 2021

No updates on this? I do think that having a pre-build container is extremely useful as most people will not need extra extensions and performance is quite important for something that runs so often.

@JulienKode
Copy link
Owner

Maybe we should release two different version of this action, currently people are using it with theirs dependency and it's not pre-build , but I agree to have a pre-build one, when you have the deps that you want inside

also caching node module install can help to make it faster

So what we can do is to have two different action:

  • one pre-build with docker
  • one that let install the deps yourself (Current behaviour of usage on projects)

@Nithos
Copy link

Nithos commented Nov 24, 2022

Is there any update on this? It would be nice to use on repos that don't contain a config file, this way it will just use the default one and avoids additional install of node.

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.

4 participants