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

EDSC-3844 Implement @edsc/eslint-config in cmr-graphql #66

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

daniel-zamora
Copy link
Contributor

@daniel-zamora daniel-zamora commented Sep 13, 2023

Overview

What is the feature?

Changes linter config

What is the Solution?

We not use shared config @edsc

What areas of the application does this impact?

N/A

Checklist

  • I have added automated tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #66 (efacf7c) into main (ca0e296) will not change coverage.
The diff coverage is 100.00%.

❗ Current head efacf7c differs from pull request most recent head dde85d6. Consider uploading reports for the commit dde85d6 to get more accurate results

@@            Coverage Diff            @@
##              main       #66   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           61        61           
  Lines         1279      1278    -1     
  Branches       165       165           
=========================================
- Hits          1279      1278    -1     
Files Changed Coverage Δ
src/cmr/concepts/concept.js 100.00% <ø> (ø)
src/cmr/concepts/dataQualitySummary.js 100.00% <ø> (ø)
src/cmr/concepts/grid.js 100.00% <ø> (ø)
src/cmr/concepts/orderOption.js 100.00% <ø> (ø)
src/datasources/graphDbDuplicateCollections.js 100.00% <ø> (ø)
src/datasources/service.js 100.00% <ø> (ø)
src/resolvers/collection.js 100.00% <ø> (ø)
src/resolvers/service.js 100.00% <ø> (ø)
src/utils/mmtQuery.js 100.00% <ø> (ø)
src/utils/parseRequestedFields.js 100.00% <ø> (ø)
... and 8 more

@daniel-zamora daniel-zamora changed the title Implement @edsc/eslint-config in cmr-graphql EDSC-3844 Implement @edsc/eslint-config in cmr-graphql Sep 13, 2023
README.md Outdated
npm install
npm install

To install the edsc shared lint configuration:
Copy link
Contributor

Choose a reason for hiding this comment

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

This step isn't necessary, it is included in the repo now and is covered by npm install

Copy link
Contributor

@trevorlang trevorlang 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 other than the README change @macrouch suggested

Copy link
Contributor

@macrouch macrouch left a comment

Choose a reason for hiding this comment

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

The max-len warnings in graphDb.test.js are annoying for the regex lines. I think we need to update the eslint config repo to ignore regex https://eslint.org/docs/latest/rules/max-len#ignoreregexpliterals

After we update that repo and deploy a new version of the npm package, and update that package here, those warnings should go away

@daniel-zamora daniel-zamora merged commit 6b5c4e6 into main Sep 14, 2023
5 checks passed
@daniel-zamora daniel-zamora deleted the EDSC-3844 branch September 14, 2023 16:33
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.

None yet

3 participants