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.

Sign up for GitHub

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

Jump to bottom

fix: correctly import js-yaml to fix yaml config loading #1033

Merged
merged 1 commit into from
Oct 26, 2021
Merged

fix: correctly import js-yaml to fix yaml config loading #1033

merged 1 commit into from
Oct 26, 2021

Conversation

iiroj
Copy link
Member

@iiroj iiroj commented Oct 24, 2021

This fixes #1032

@iiroj
Copy link
Member Author

iiroj commented Oct 24, 2021

No idea why the tests don't handle this, but I assume they're not using yaml config files. Pls review @okonet

You can see here that the default export is jsYaml, and that it contains the jsYaml.load method:

https://github.com/nodeca/js-yaml/blob/49baadd52af887d2991e2c39a6639baa56d6c71b/dist/js-yaml.js#L3863

https://github.com/nodeca/js-yaml/blob/49baadd52af887d2991e2c39a6639baa56d6c71b/dist/js-yaml.js#L3846

@iiroj iiroj requested a review from okonet October 24, 2021 15:53
@iiroj
Copy link
Member Author

iiroj commented Oct 24, 2021

This was broken in #981

@iiroj iiroj mentioned this pull request Oct 24, 2021
Closed
@codecov
Copy link

codecov bot commented Oct 24, 2021 •
edited
Loading

Codecov Report

Merging #1033 (c2d3e4f) into master (04529e2) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1033   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        20           
  Lines          656       656           
  Branches       149       149           
=========================================
  Hits           656       656
Impacted Files Coverage Δ
lib/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04529e2...c2d3e4f. Read the comment docs.

@iiroj
Copy link
Member Author

iiroj commented Oct 26, 2021

Ping @okonet this should get merged.

@okonet
Copy link
Collaborator

okonet commented Oct 26, 2021

Let's merge since it was approved already.

okonet
okonet reviewed Oct 26, 2021
View reviewed changes
Copy link
Collaborator

@okonet okonet left a comment

Choose a reason for hiding this comment

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

A test would be awesome to not have this regression in the future.

@iiroj
Copy link
Member Author

iiroj commented Oct 26, 2021

@okonet yeah I will add tests separately, but let's fix this first.

@iiroj iiroj merged commit 612d806 into master Oct 26, 2021
@iiroj iiroj deleted the fix-yaml-config branch October 26, 2021 11:48
@github-actions
Copy link
Contributor

🎉 This PR is included in version 11.2.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

iiroj added a commit that referenced this pull request Oct 26, 2021
@iiroj
Revert "fix: correctly import js-yaml to fix yaml config loading (#…
9e7d851 
…1033)"

This reverts commit 612d806.
@iiroj iiroj mentioned this pull request Oct 26, 2021
Merged
iiroj added a commit that referenced this pull request Oct 26, 2021
@iiroj
fix: revert back to cosmiconfig from lilconfig (#1035)
e035b80 
* Revert "fix: correctly import `js-yaml` to fix yaml config loading (#1033)"

This reverts commit 612d806.

* Revert "perf: replace `cosmiconfig` with `lilconfig` (#981)"

This reverts commit 04529e2.
@github-actions
Copy link
Contributor

🎉 This PR is included in version 11.3.0-beta.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sync-by-unito sync-by-unito bot mentioned this pull request Nov 1, 2021
Closed
@iiroj iiroj mentioned this pull request Nov 13, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@okonet okonet okonet approved these changes

@kulek1 kulek1 kulek1 approved these changes

Assignees
No one assigned
Labels
released on @beta released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

v11.2.4 is broken for config .lintstagedrc
3 participants
@iiroj @okonet @kulek1