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

generator: add support for remote esLint config #100

Merged
merged 1 commit into from
Apr 24, 2021

Conversation

blackfalcon
Copy link
Member

Alaska Airlines Pull Request

Fixes: #62

Summary:

The scope of this PR removes locally installed eslint configs and update to support the new remote eslint-config

Type of change:

Please delete options that are not relevant.

  • New capability
  • Revision of an existing capability
  • Infrastructure change (automation, etc.)
  • Other (please elaborate)

Checklist:

  • My update follows the CONTRIBUTING guidelines of this project
  • I have performed a self-review of my own update

By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.

Thank you for your submission!

-- Auro Design System Team

@blackfalcon blackfalcon added the Type: Feature New Feature label Apr 17, 2021
@blackfalcon blackfalcon requested a review from geoffrich April 17, 2021 04:47
@blackfalcon blackfalcon self-assigned this Apr 17, 2021
@blackfalcon blackfalcon linked an issue Apr 17, 2021 that may be closed by this pull request
@geoffrich geoffrich changed the title generator: add support for remote csLint config generator: add support for remote esLint config Apr 21, 2021
Copy link
Contributor

@geoffrich geoffrich left a comment

Choose a reason for hiding this comment

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

Two questions:

  • Is there a reason our config is now in .eslintrc instead of .eslintrc.js? I'm not aware of the implications, but it seems like the docs recommend .eslintrc.js.
  • Did we verify that this config works (i.e. scaffold a new component and you can trigger lint errors)?

Otherwise, this PR was a lot less intimidating than I was expecting 😅

@blackfalcon blackfalcon force-pushed the dsande/generator#62/eslint branch from 04f6210 to bfa4f76 Compare April 22, 2021 06:40
@blackfalcon
Copy link
Member Author

  • Is there a reason our config is now in .eslintrc instead of .eslintrc.js? I'm not aware of the implications, but it seems like the docs recommend .eslintrc.js.

The .eslintrc is only in the repo to reference the npm's .eslintrc.js

  • Did we verify that this config works (i.e. scaffold a new component and you can trigger lint errors)?

oh fo sho

@geoffrich
Copy link
Contributor

If the stylelint config moves into the config package as well, I think it should be called something more generic, e.g. @aurodesignsystem/config. What do you think? If we change the name, we should make this PR use the new package before merging.

@blackfalcon
Copy link
Member Author

If the stylelint config moves into the config package as well...

We have to test the scenario out to see if it all works that way. The esLint docs specifically asked for a naming convention, but I am not sure how dependent it is on that convention.

But if it works, then I have no issue with the rename.

@blackfalcon
Copy link
Member Author

So, one step at a time here. Let's get this PR and the Sass lint update PR all merged and then move into the next step. Trying to juggle these PRs that work AND other decisions on the fly that may not work... too confusing.

@blackfalcon blackfalcon merged commit 03462d6 into next Apr 24, 2021
@blackfalcon blackfalcon deleted the dsande/generator#62/eslint branch April 24, 2021 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Centralize and simplify ESLint config
2 participants