Discussion: custom configuration

[z0al]

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

[marionebl]

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

1. Look up .?commitlint.{js,yml.json} and patternplate in package.json via the Github API instead of FS
2. 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
3. Resolve relative .extends and .parserPreset, goto 2
4. Use absolute .extends and .parserPreset if they are found in a defined whitelist

cc @paulirish

[z0al]

@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?

[marionebl]

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

[marionebl]

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

[marionebl]

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

[z0al]

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 :/

[z0al]

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?

[marionebl]

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?

[marionebl]

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.

[paulirish]

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.

[z0al]

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

[z0al]

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

[marionebl]

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

[z0al]

@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!

[z0al]

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!

[marionebl]

@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);
  }
}

[marionebl]

@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/

[z0al]

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

[marionebl]

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

[z0al]

Gitter sounds good, which channel?

[marionebl]

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"]}}}

[z0al]

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?

[z0al]

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

[marionebl]

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.

[z0al]

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 ;)

[z0al]

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)

[caarlos0]

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

[fathyb]

@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.

[bennypowers]

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

[z0al]

@bennypowers Yeah, that would be awesome.

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

[bennypowers]

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

[wagoid]

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).

[bennypowers]

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