Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

Add eslint plugin yml to enable correct YAML parsing#2868

Merged
nschonni merged 13 commits intonodejs:mainfrom
lyqht:feature/lint-yml
Oct 4, 2022
Merged

Add eslint plugin yml to enable correct YAML parsing#2868
nschonni merged 13 commits intonodejs:mainfrom
lyqht:feature/lint-yml

Conversation

@lyqht
Copy link
Copy Markdown
Contributor

@lyqht lyqht commented Oct 4, 2022

Description

While working on #2854, I realized that linting & formatting is not applied to the .yml files. Hence this PR adds them.

Example of invalid YAML file giving 1 error

image

Notes

There are a few available yml configurations as per the plugin, but I applied the yml:base configuration for now as currently there are a few files that would fail the recommended/standard configuration (the 70+ errors above). It's up to the maintainers if they prefer those instead.

P.S. I didn't create an issue, but I hope this get a hacktoberfest-accepted label 😆

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npm run lint:js -- --fix and/or npm run lint:md -- --fix for my JavaScript and/or Markdown changes.
    • This is important as most of the cases your code changes might not be correctly linted
  • I have run npm run test to check if all tests are passing, and/or npm run test -- -u to update snapshots if I created and/or updated React Components.
  • I have checked that the build works locally and that npm run build work fine.
  • I've covered new added functionality with unit tests if necessary.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Oct 4, 2022

Codecov Report

Base: 66.02% // Head: 63.82% // Decreases project coverage by -2.20% ⚠️

Coverage data is based on head (adbb194) compared to base (fd57b87).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2868      +/-   ##
==========================================
- Coverage   66.02%   63.82%   -2.21%     
==========================================
  Files         118      124       +6     
  Lines        1351     1465     +114     
  Branches      342      367      +25     
==========================================
+ Hits          892      935      +43     
- Misses        422      490      +68     
- Partials       37       40       +3     
Impacted Files Coverage Δ
src/__fixtures__/hooks.tsx 100.00% <ø> (ø)
src/__fixtures__/page.tsx 94.11% <ø> (-5.89%) ⬇️
...components/ApiComponents/Components/ApiHeading.tsx 0.00% <ø> (ø)
...rc/components/ApiComponents/Components/ApiLink.tsx 0.00% <ø> (ø)
...ents/ApiComponents/Components/SourceLink/index.tsx 0.00% <ø> (ø)
...ApiComponents/Components/VersionSelector/index.tsx 0.00% <ø> (ø)
src/components/ApiComponents/index.tsx 0.00% <ø> (ø)
src/components/Article/index.tsx 100.00% <ø> (ø)
src/components/Banner/index.tsx 100.00% <ø> (ø)
src/components/Codebox/index.tsx 100.00% <ø> (ø)
... and 80 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Copy Markdown
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

The plugin and changes make sense to me.

@nschonni nschonni dismissed their stale review October 4, 2022 16:21

Plug-ins have been cleaned out. Only a minor suggestion remains

lyqht and others added 2 commits October 5, 2022 00:34
Co-authored-by: Nick Schonning <nschonni@gmail.com>
Signed-off-by: Estee Tey Siew Wen <estee.tsw@gmail.com>
Signed-off-by: Nick Schonning <nschonni@gmail.com>
Signed-off-by: Nick Schonning <nschonni@gmail.com>
Signed-off-by: Nick Schonning <nschonni@gmail.com>
Signed-off-by: Nick Schonning <nschonni@gmail.com>
@nschonni
Copy link
Copy Markdown
Member

nschonni commented Oct 4, 2022

@lyqht I pushed a few commits to tie the check back to the CI. It's flagging one file if you want to run the format again locally to address the last file

@lyqht
Copy link
Copy Markdown
Contributor Author

lyqht commented Oct 4, 2022

@nschonni haha i hope that's the 1 file you're talking about 😆

@nschonni nschonni merged commit 304a1e2 into nodejs:main Oct 4, 2022
@ovflowd ovflowd added the hacktoberfest-accepted Used to label PR's label Oct 4, 2022
@ovflowd
Copy link
Copy Markdown
Member

ovflowd commented Oct 4, 2022

@nschonni don't forget please to add the hacktoberfest-accepted PRs for hacktoberfest PRs 🙈

@nschonni
Copy link
Copy Markdown
Member

nschonni commented Oct 4, 2022

You don't actually need the label if the repo has the tag and the PR is merged https://hacktoberfest.com/participation/#pr-mr-details. The label doesn't hurt though, and I have slapped it on a few, since it might speed up the PRs being counted as accepted. Otherwise, merged PRs get counted after 7 days

@ovflowd
Copy link
Copy Markdown
Member

ovflowd commented Oct 4, 2022

Oooh, that I didn’t know 👀 thanks for explaining!

@nschonni
Copy link
Copy Markdown
Member

nschonni commented Oct 4, 2022

The tag is also helpful if you want it to count, but might not be able to land it before the event ends

@ovflowd
Copy link
Copy Markdown
Member

ovflowd commented Oct 4, 2022

Thank you, Nick! That’s very informative 🙏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

hacktoberfest-accepted Used to label PR's

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants