Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[chore] Update eslint dep #2000

Merged
merged 10 commits into from
Dec 4, 2019
Merged

[chore] Update eslint dep #2000

merged 10 commits into from
Dec 4, 2019

Conversation

sirugh
Copy link
Contributor

@sirugh sirugh commented Nov 22, 2019

Description

This is a follow up to the magento-eslint security issue. I updated our usage to v1.5.0 and in doing so had to handle a bunch of new warnings/errors. I also had to update some peer dependencies and install the config as a dependency to buildpack and peregrine where it was not before.

Question: Should this be a patch? People using it with custom components may start getting warnings for things.

Related Issue

Closes PWA-193.

Acceptance

Verification Stakeholders

@jimbo @zetlen

Specification

Verification Steps

  1. yarn && yarn lint, no errors.
  2. Make sure all the disables make sense. If they don't let's fix it here.

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have updated the documentation accordingly, if necessary.
  • I have added tests to cover my changes, if necessary.

@sirugh sirugh added the version: Patch This changeset includes backwards compatible bug fixes. label Nov 22, 2019
@sirugh sirugh requested review from zetlen and jimbo November 22, 2019 17:50
@sirugh sirugh added the tooling Related to the developer tooling, including buildpack, webpack plugins, and debug UIs. label Nov 22, 2019
@PWAStudioBot PWAStudioBot added pkg:peregrine pkg:pwa-buildpack pkg:upward-js Pertains to upward-js reference implementation of UPWARD. pkg:upward-spec Pertains to UPWARD specification package. pkg:venia-concept labels Nov 22, 2019
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Nov 22, 2019

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Associated JIRA tickets: PWA-193.

Generated by 🚫 dangerJS against 88cfd66

revanth0212
revanth0212 previously approved these changes Nov 22, 2019
rules: {
'no-undef': 'off',
'no-useless-escape': 'off'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have to define these two rules everywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we disregard them throughout the application. The alternative would be to add these rules to magento-eslint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added these to magento-eslint and published but I'm reconsidering that... I don't think we should be disabling no-undef. We should be updating some config with those values somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created PWA-207.

jimbo
jimbo previously approved these changes Nov 26, 2019
@sirugh sirugh added the hold On hold until another condition is fulfilled. label Nov 26, 2019
@sirugh sirugh removed the hold On hold until another condition is fulfilled. label Nov 27, 2019
jimbo
jimbo previously approved these changes Dec 3, 2019
zetlen
zetlen previously approved these changes Dec 3, 2019
Copy link
Contributor

@zetlen zetlen left a comment

Choose a reason for hiding this comment

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

Approved, because updating all our code to satisfy decent lint rules is important, but not so important right now that we want all that code churn.

@sirugh sirugh dismissed stale reviews from zetlen and jimbo via 88cfd66 December 4, 2019 21:19
@tjwiebell tjwiebell merged commit 4c00165 into develop Dec 4, 2019
@tjwiebell tjwiebell deleted the update-eslint-dep branch December 4, 2019 21:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
pkg:peregrine pkg:pwa-buildpack pkg:upward-js Pertains to upward-js reference implementation of UPWARD. pkg:upward-spec Pertains to UPWARD specification package. pkg:venia-concept pkg:venia-ui tooling Related to the developer tooling, including buildpack, webpack plugins, and debug UIs. version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants