Discussion: custom configuration

Question

Discussion: custom configuration

z0al opened this issue 8 years ago · 33 comments

I would like to discuss what things I need to consider when supporting custom configuration files in the root repository directory.

More specifically, how can we support .commitlint.yml or .commitlint.json ? I can't see any way to support .js based config files

Any input is welcome!
cc @marionebl

Answer 1 · 2017-11-04T07:56:13.000Z

I rolled this around a bit already here are my thoughts:

Look up .?commitlint.{js,yml.json} and patternplate in package.json via the Github API instead of FS
Load the contents of first match via Github API
3a) .yml and .json: parse the files
3b) .js: Idea: Run the script in a safe node sandbox with most features and APIs (e.g. require) disabled/limited
Resolve relative .extends and .parserPreset, goto 2
Use absolute .extends and .parserPreset if they are found in a defined whitelist

cc @paulirish

Answer 2 · 2017-11-04T15:13:27.000Z

@marionebl Thank you for sharing this

Look up .?commitlint.{js,yml.json} and patternplate in package.json via the Github API instead of FS

I have no idea what patternplate is and how it's related to commitlint?

Answer 3 · 2017-11-04T15:48:12.000Z

Whoops, I meant commitlint, referring to the feature of cosmiconfig where config is read from a section in package.json

Answer 4 · 2017-11-04T15:53:16.000Z

I do not have a good solution for making this work with things like commitlint-config-lerna though without cloning the repository.

Answer 5 · 2017-11-04T15:57:19.000Z

I understand correctly now.sh is the primary deployment target for probot?

Answer 6 · 2017-11-04T16:00:05.000Z

I think a command line approach ( like how bundlesize works) may fits things like commitlint-config-lerna

I understand correctly now.sh is the primary deployment target for probot?

I may only use now.sh - at least for now. Other services are forbidden in my country :/

Answer 7 · 2017-11-04T16:41:06.000Z

Here is a another approach:

Standalone CLI
Only runs in a CI (i.e. Travis, Circle .etc) environment. It executes commitlint targeting the pre-cloned repo and send the report to our API endpoint! Done (similar to the way code coverage services like https://codecov.io work?)
The web service (the current bot)
Its only function is to analyze the report then update the PR status accordingly! it wouldn't be necessary listen to GitHub's webhooks anymore

What do you think?

Answer 8 · 2017-11-04T18:09:25.000Z

Sounds like a very good idea! This would require json formatted output for commitlint and some tool to manage the transport to commitlint-bot right?

Answer 9 · 2017-11-04T18:13:06.000Z

I'd add the complete resolved config to the json output. This way we could even cover @paulirish's use case of linting the PR title, using said resolved config from CI runs to check on title changes indicated by web hooks.

Answer 10 · 2017-11-04T19:49:00.000Z

yo guys! haha it's another commitlint bot! :D

my project is, yah, only focused on the PR title and doesn't care about commits. As such, it wouldn't make sense to be a CLI that runs inside of CI (like codecov or bundlesize). Because a user needs to iterate on the PR title a few times to get it right.. so it needs to be webhook.

In mine, I pull the commitlint.config.js from the root of the repo: https://github.com/paulirish/commitlintbot/blob/9368713fa237a983155d9f57906d0797931e9063/index.js#L33-L37 a little janky but it works.

Answer 11 · 2017-11-05T07:25:46.000Z

In mine, I pull the commitlint.config.js from the root of the repo

@paulirish umm, it may not work for commitlint-config-lerna though

Answer 12 · 2017-11-05T07:30:16.000Z

It may also limit the possible supported scenarios?

However, you're right, running inside a CI wouldn't make sense if the user only wants title linting

Answer 13 · 2017-11-08T20:59:57.000Z

I think we can have the cake and eat it, too - by taking up @ahmed-taj's idea to send information from CI to commitlint-bot.

I propose to not lint on CI, but let commitlint send the resolved config for a CI run to commitlint-bot.

Step by step this would work like this:

1. Linting a PR title

PR is opened: commitlint-bot waits for resolved config to be sent
CI runs commitlint (sans actual linting), sends fully resolved config to commitlint-bot. This may include stuff like the results of commitlint-config-lerna.
Using the resolved config, commitlint-bot lints the given title
On commits/history changes: Wait for new resolved config to be sent
On title changes: Wait for any pending configs, then lint the title contents

2. Linting a PR commit range

PR is opened: commitlint-bot waits for resolved config to be sent
CI sends fully resolved commitlint config to commitlint-bot
Using the resolved config, commitlint-bot lints al commits in the PR
On commits/history changes: Wait for new resolved config to be sent, then lint
On title changes: Nothing happens

On a side note: Would it make sense to check the current merge strategy of a PR (if possible) and automatically do the right thing, e.g.:

Merge: Do not lint
Rebase: Lint all commits
Squash: Lint the PR title

Answer 14 · 2017-11-11T16:41:13.000Z

@marionebl sounds very good 👍 , I think I will go with your proposal!

I will try to work on this in the next couple of days. Will open a WIP pull request.

On a side note: Would it make sense to check the current merge strategy of a PR (if possible)

AFAIK, GitHub doesn't provide an API for this, yet!

Answer 15 · 2017-11-11T17:17:23.000Z

Oh, that needs an option in commitlint to output resolved config as a JSON. AFAIK commitlint doesn't support this yet!

@marionebl you can point me where to start to add this functionality to @commitlint/cli, I may give it a try!

Answer 16 · 2017-11-11T17:35:50.000Z

@ahmed-taj Yeah, we'll have to add this as subcommand to commitlint/cli, e.g.:

commitlint config --resolve --json # machine output
commitlint config --resolve # human readable output, let's implement later

Just before @commitlint/cli/src/cli.js#L80 something like this could go:

const configCommand = require('./commands/config');
const defaultCommand = require('./commands/default');

async function main(options) {
  const raw = options.input;
  const command = raw.length > 1 ? raw[0] : null;
  // ...
  switch(command) {
    case 'config':
       return configCommand(options);
    default:
       return defaultCommand(options);
  }
}

Answer 17 · 2017-11-13T19:47:41.000Z

@ahmed-taj I noticed you went ahead and forked commitlint. Don't hesitate to contact me if you hit road blocks or just could do with some help.

Ping me e.g. on the commitlint channel in the devtools slack:
http://devtoolscommunity.herokuapp.com/

Answer 18 · 2017-11-14T08:26:09.000Z

Hey @marionebl I appreciate it :)

Sadly, I can no longer use Slack

But I can open a work-in-progress pull request in your commitlint repo so we may continue discussion there

Answer 19 · 2017-11-14T09:40:45.000Z

Ok, that's a bummer. What Messengers do you use? I am e.g. on Gitter. Can make IRC happen too.

Answer 20 · 2017-11-14T17:04:16.000Z

Gitter sounds good, which channel?

Answer 21 · 2017-12-04T11:21:31.000Z

I published a pre-release of the required commitlint functionality, you can pull it in via

npm install @commitlint/cli@next

Invoking commitlint config --format=json in a directory should produce stdout output like this. The example is produced by executing the prerelease version in the root of commitlint project.

commitlint config --json
{"extends":["./@commitlint/config-conventional","./@commitlint/config-lerna-scopes"],"rules":{"scope-enum":[2,"always",["commitlint-config-angular","commitlint-config-lerna-scopes","commitlint-config-patternplate","commitlint","cli","config-angular-type-enum","config-angular","config-conventional","config-lerna-scopes","config-patternplate","core","prompt-cli","prompt","travis-cli","babel-preset-commitlint","example-prompt","test","utils"]]},"parserPreset":{"name":"conventional-changelog-angular","path":"/Users/marneb/Documents/oss/commitlint/node_modules/conventional-changelog-angular/index.js","opts":{"headerPattern":{},"headerCorrespondence":["type","scope","subject"],"noteKeywords":["BREAKING CHANGE","BREAKING CHANGES"],"revertPattern":{},"revertCorrespondence":["header","hash"]}}}

Answer 22 · 2017-12-04T12:06:33.000Z

Great work @marionebl

Invoking commitlint config --json in a directory should ...

Plain --json doesn't work, you need to set the value to "format", i.e. --format=json

So, about the path key should be absolute?

Answer 23 · 2017-12-04T12:12:08.000Z

I think the next step is to fully resolve the configs, so we end up having no need for extends part, right?

Answer 24 · 2017-12-04T12:18:14.000Z

Great work @marionebl

Invoking commitlint config --json in a directory should ...

Plain --json doesn't work, you need to set the value to "format", i.e. --format=json

So, about the path key should be absolute?

Yep, sorry about that - I edited my comment to reflect this.

I think the next step is to fully resolve the configs, so we end up having no need for extends part, right?

The config actually is fully resolved (the extensions are expanded into .rules), the .extends key just isn't very useful yet.

Answer 25 · 2017-12-04T12:38:16.000Z

The config actually is fully resolved (the extensions are expanded into .rules), the .extends key just isn't very useful yet.

Great! I may just ignore .extends for now.

We now have the "cake", let's eat it ;)

Answer 26 · 2017-12-04T12:41:15.000Z

I think I start the logic on the bot side the next couple of days (when I have a final idea on how to "authorize" the upload request)

Answer 27 · 2017-12-06T02:36:31.000Z

reading from .github/config.yml may also be a good idea :)

Answer 28 · 2017-12-07T12:51:06.000Z

@ahmed-taj I can help if needed to run untrusted JS code on Node, as the vm Node module is not considered safe (see the node on the module page).

If the code does not contain any require or other APIs (pure, vanilla ECMAScript) we can make a lightweight isolated v8 VM using v8::Isolate.

Answer 29 · 2019-03-14T21:46:36.000Z

A bit of a flyer, but how about a commitlint github action that just runs commitlint cli in the repo?

Answer 30 · 2019-03-15T08:25:36.000Z

@bennypowers Yeah, that would be awesome.

I think about making one but didn't get early access to Github Actions yet.

Answer 31 · 2019-03-15T09:30:23.000Z

See https://github.com/bennypowers/commitlint-gh-actions (untested.. not even sure if it will run) and conventional-changelog/commitlint#586

Answer 32 · 2019-10-08T13:15:33.000Z

I've created a github action for this case, if you want to have a look: https://github.com/marketplace/actions/commit-linter

I tried using @bennypowers's action but got some issues and ended up creating another one, as explained here.

This one was made to not depend on the project's stack, so it comes with a set of shared configs available to use. The downside is that people can't use configs that come from a private npm package, but I think this is way less likely to happen. If someone needs that case, we can also create another commitlint action that depends on context (or maybe update Benny's one to do that).

Answer 33 · 2019-10-10T09:36:44.000Z

Thanks @wagoid for publishing. Happy to take PRs, as I've not had the time to invest in this recently.