Bump cosmiconfig version and conditionally support mjs config#3747
Bump cosmiconfig version and conditionally support mjs config#3747escapedcat merged 15 commits intoconventional-changelog:masterfrom
Conversation
…pendency for @commitlint/load
| image: 'ubuntu:22.04' | ||
| build: | ||
| strategy: | ||
| matrix: |
There was a problem hiding this comment.
Simplified this with the build matrix so they all run the same steps, but @escapedcat do I need to keep the original checks around temporarily?
I did this to take advantage of the fact that the setup node action pulls down node 20.9.0, whereas the original job pulled down only 20.5.x and I didn't see a way to configure it.
There was a problem hiding this comment.
I think pulling latest v20 is fine.
Thanks for introducing the matrix. Simplifies things in the future.
I might need to adjust the required checks in the settings. Will check.
| }); | ||
| }); | ||
|
|
||
| const mjsConfigFiles = isDynamicAwaitSupported() |
There was a problem hiding this comment.
New set of tests for each config type. I removed the older tests and their fixture data, but verified they were still passing in an older commit.
| const configPath = path.join(__dirname, `../fixtures/config/${configFile}`); | ||
| const config = readFileSync(configPath); | ||
|
|
||
| writeFileSync(path.join(cwd, configFile), config); |
There was a problem hiding this comment.
This test.each writes a config file from fixtures/config to the template directory and then loads the found config from the current working directory. Everything except ts files should be supported by the template directory.
| image: 'ubuntu:22.04' | ||
| build: | ||
| strategy: | ||
| matrix: |
There was a problem hiding this comment.
I think pulling latest v20 is fine.
Thanks for introducing the matrix. Simplifies things in the future.
I might need to adjust the required checks in the settings. Will check.
|
If you don't mind you could introduce the matrix checks in a separate PR. We could merge that first, adjust required checks and merge this after. |
|
Thanks a lot for this @joberstein ! ❤️ |
Sure, I'll make a separate one for that first. |
Link to separate PR is: #3748 |
|
Merged the matrix PR, wanna rebase this? |
|
@escapedcat I finished manual testing (added a description in the first comment) and updated the docs. |
|
Impressive, thanks! Will merge and release |
|
Looks like this breaks our CI: https://github.com/mikro-orm/mikro-orm/actions/runs/6823592237/job/18558301519 I can't reproduce this locally now. |
|
@B4nan I cloned the master branch of your repo and reproduced with a test commit. I noticed that cosmiconfig is at 8.0.0 in its package.json in node_modules though, which is a version from before they added support for mjs files. Going to look into why this version is being installed instead of ^8.0.0 (v8.3.6). |
|
Oh I see, tried to deduplicate the lock file, lets see if that helps |
Apparently it's an issue with yarn (maybe npm also?) in that it won't re-install transitive dependencies to meet semver requirements. So even though commitlint/load is using ^8.0.0, since the consuming repo already has installed it as 8.0.0, it won't update unless the commitlint/load cosmiconfig version changes to something compatible (in this case ^8.2.0). I'd like to use an exact version though, so it won't be assumed to update for dependent projects. I think what you have should work as well though. |
Bump cosmiconfig version and conditionally support mjs config. Also simplify CI test matrix and add more tests for the different config files.
Draft until I do manual testing + need to update docs too.
Description
This PR:
Motivation and Context
Replacement PR for automated PR: #3654
Also resolves: #3611
Usage examples
With node >= 20.8.0, a user can now specify commitlint config in '.commitlintrc.mjs' or 'commitlint.config.mjs'.
How Has This Been Tested?
Types of changes
Checklist: