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.

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

Adds a YAML eslint linter. #312

Merged
merged 6 commits into from
Jun 7, 2024
Merged

Conversation

dblock
Copy link
Member

@dblock dblock commented Jun 4, 2024

Description

Lints YAML using a formatting linter. Fixes things like spaces, preferring single/double quotes, etc.

We have a spec linter that is about the spec itself in which we could implement some of these rules too, but feels easier. That one probably could have been (still can be) a generic eslint linter, too.

Issues Resolved

Closes #266.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link
Contributor

github-actions bot commented Jun 4, 2024

Changes Analysis

Commit SHA: ae4c918
Comparing To SHA: 6c30b32

API Changes

Summary

NO CHANGES

Report

The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/9416605489/artifacts/1578965962

API Coverage

Before After Δ
Covered (%) 476 (46.62 %) 476 (46.62 %) 0 (0 %)
Uncovered (%) 545 (53.38 %) 545 (53.38 %) 0 (0 %)
Unknown 24 24 0

@dblock dblock marked this pull request as ready for review June 4, 2024 13:47
@@ -51,7 +68,22 @@ export default [
'array-callback-return': 'off',
'new-cap': 'off',
'no-return-assign': 'error',
'object-shorthand': 'error'
'object-shorthand': 'error',
'no-constant-condition': 'off'
Copy link
Collaborator

Choose a reason for hiding this comment

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

We're turning off a lot of TS lints in this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually we're not, these are added via recommended-type-checked and I believe we didn't have them enabled before.

$ git diff
diff --git a/eslint.config.mjs b/eslint.config.mjs
index 53e6a18..acb7a4b 100644
--- a/eslint.config.mjs
+++ b/eslint.config.mjs
@@ -52,6 +52,13 @@ export default [
           allowRuleToRunWithoutStrictNullChecksIKnowWhatIAmDoing: false
         }
       ],
+      '@typescript-eslint/no-explicit-any': 'error',
+      '@typescript-eslint/no-unsafe-assignment': 'error',
+      '@typescript-eslint/no-unsafe-return': 'error',
+      '@typescript-eslint/no-unsafe-call': 'error',
+      '@typescript-eslint/no-unsafe-member-access': 'error',
+      '@typescript-eslint/no-unused-vars': ['error', { "argsIgnorePattern": "^_" }],
+      '@typescript-eslint/require-await': 'error',
       'array-callback-return': 'off',
       'new-cap': 'off',
       'no-return-assign': 'error',
$ npm run lint

✖ 193 problems (193 errors, 0 warnings)
  3 errors and 0 warnings potentially fixable with the `--fix` option.

Copy link
Member Author

Choose a reason for hiding this comment

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

Two of these are easy to enable. The others give hundreds of errors.

@dblock dblock marked this pull request as draft June 5, 2024 20:05
@dblock dblock force-pushed the yaml-lint branch 2 times, most recently from 8784e29 to 92d0ca2 Compare June 6, 2024 13:18
@dblock dblock marked this pull request as ready for review June 6, 2024 13:19
@dblock
Copy link
Member Author

dblock commented Jun 6, 2024

This is ready. I only auto-fixed yml/indent and turned off single/double quote rules that remove quotes from keys.

nhtruong
nhtruong previously approved these changes Jun 6, 2024
Copy link
Collaborator

@nhtruong nhtruong left a comment

Choose a reason for hiding this comment

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

Looks good to me. Need a rebase.

@dblock
Copy link
Member Author

dblock commented Jun 6, 2024

Looks good to me. Need a rebase.

Rebased.

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
…e linter.

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: dblock <dblock@amazon.com>
Copy link

codecov bot commented Jun 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@6c30b32). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #312   +/-   ##
=======================================
  Coverage        ?   88.74%           
=======================================
  Files           ?       40           
  Lines           ?     1164           
  Branches        ?      283           
=======================================
  Hits            ?     1033           
  Misses          ?      123           
  Partials        ?        8           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: dblock <dblock@amazon.com>
@nhtruong nhtruong merged commit 64b747c into opensearch-project:main Jun 7, 2024
10 checks passed
@dblock dblock deleted the yaml-lint branch June 7, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add text quote linter.
2 participants