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

fix(lint): Change endOfLine rules to better support linting on Windows #20078

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

HowardBraham
Copy link
Contributor

Before this PR, if you run eslint on Windows, you will get thousands of CRLF errors like this:

    1:52   error  Delete `␍`  prettier/prettier
    2:52   error  Delete `␍`  prettier/prettier
    3:74   error  Delete `␍`  prettier/prettier

You can fix them with yarn lint:fix, but they will probably come back again, depending on your git and text editor settings.

Meanwhile, git commits everything as LF, so it shouldn't affect Mac/Linux.

Notes:

  • Made similar changes to eslint-config, but metamask-extension is currently on an old version of eslint-config, so putting it in this repo as well
    fix(lint): Change endOfLine rules to better support linting on Windows eslint-config#311
  • This line in package.json was invalid syntax: "lint:styles": "stylelint -- */**/*.scss",
    and on Windows, produced the error The command line is too long.
    Changed to the valid "lint:styles": "stylelint '*/**/*.scss'",

@HowardBraham HowardBraham requested a review from a team as a code owner July 18, 2023 18:09
@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot
Copy link
Collaborator

Builds ready [638858b]
Page Load Metrics (1536 ± 38 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1031741282311
domContentLoaded1417176015367938
load1418176015367938
domInteractive1417176015367938
Bundle size diffs
  • background: 0 bytes
  • ui: 0 bytes
  • common: 0 bytes

@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #20078 (638858b) into develop (e75535d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop   #20078   +/-   ##
========================================
  Coverage    69.43%   69.43%           
========================================
  Files          990      990           
  Lines        37428    37428           
  Branches     10039    10039           
========================================
  Hits         25986    25986           
  Misses       11442    11442           

@@ -63,7 +63,7 @@
"lint:lockfile:dedupe:fix": "yarn dedupe",
"lint:lockfile": "lockfile-lint --path yarn.lock --allowed-hosts npm yarn github.com codeload.github.com --empty-hostname true --allowed-schemes \"https:\" \"git+https:\" \"npm:\" \"patch:\" \"workspace:\"",
"lint:shellcheck": "./development/shellcheck.sh",
"lint:styles": "stylelint -- */**/*.scss",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change necessary because lint wasn't running at all on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the second bullet point in the PR description: "on Windows, produced the error The command line is too long."

Copy link
Contributor

@mcmire mcmire 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.

@HowardBraham HowardBraham merged commit 39b1996 into develop Jul 19, 2023
@HowardBraham HowardBraham deleted the fix/windows-lint branch July 19, 2023 19:11
@github-actions github-actions bot locked and limited conversation to collaborators Jul 19, 2023
@metamaskbot metamaskbot added the release-10.36.0 Issue or pull request that will be included in release 10.36.0 label Jul 19, 2023
@Gudahtt Gudahtt added release-11.1.0 Issue or pull request that will be included in release 11.1.0 and removed release-10.36.0 Issue or pull request that will be included in release 10.36.0 labels Sep 19, 2023
@HowardBraham HowardBraham added the contributor experience An issue that impacts, or planned improvement to, the contributor experience. label Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
contributor experience An issue that impacts, or planned improvement to, the contributor experience. release-11.1.0 Issue or pull request that will be included in release 11.1.0 team-accounts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants