chicagomaroon / data-visualizations

This repository contains and hosts interactive visuals used in The Chicago Maroon publication.
https://chicagomaroon.com/
4 stars 2 forks source link

Update linter configuration #94

Closed air-kyi closed 2 months ago

air-kyi commented 2 months ago

What does this PR do?

Ran npx @eslint/migrate-config .eslintrc.json --commonjs to migrate eslint to the new version because we were getting the message Unknown file extension .json both on local machine and in the Github Action pull request checks and this was the solution suggested by npx itself.

AIR+kyi@DESKTOP-ORESNN2 MINGW64 ~/OneDrive - AIR/Documents/GitHub/data-visualizations (lint-update)
$ npm run lint

> chicagomaroon.github.io@1.0.0 lint
> npx prettier --loglevel silent --write . && npx eslint --fix .

Oops! Something went wrong! :(

ESLint: 9.4.0

ESLint couldn't find an eslint.config.(js|mjs|cjs) file.

From ESLint v9.0.0, the default configuration file is now eslint.config.js.
If you are using a .eslintrc.* file, please follow the migration guide
to update your configuration file to the new format:

https://eslint.org/docs/latest/use/configure/migration-guide

If you still have problems after following the migration guide, please stop by
https://eslint.org/chat/help to chat with the team.

Screenshot of Github Action run: image

Also changes the eslint config file from .eslintrc.json to the new file format, eslint.config.cjs. Note: this does not seem to have a marked effect on the code on this branch so we need to commit this and see if it works on incoming code?

How was the functionality tested and verified?

All steps should be completed in the order presented, and you should not move on to the next step until the previous is completed. - [ ] The title of the html page is the same as the chart. - [ ] The meta_data.json file is filled in correctly (the description is a complete sentence, the title is the same as the visual). - [ ] The chart and its associated pages have been minified using the npm run process-visual command. - [ ] The visual is styled similar to other charts (centered title/subtitle, no legends unless absolutely necessary, etc.) - [ ] This visual was approved by the lead developer. - [ ] The visual was approved by the story's stakeholders.

michplunkett commented 2 months ago

Great work, @air-kyi!

Here are some changes I think that would make this complete:

air-kyi commented 2 months ago

Great work, @air-kyi!

Here are some changes I think that would make this complete:

  • Run npm update eslint-config-prettier and npm eslint-plugin-prettier. That should make some changes to the package.json and package-lock.json, and you should commit those.
  • Make sure the build passes.
  • Update the name of the PR to the correct tense (i.e., Update linter configuration).

Thank you Michael!! Updating package.json worked but npm didn't recognize the command npm eslint-plugin-prettier fyi, and no internet results turn up anything for this command so I'm not sure it's a versioning issue?

michplunkett commented 2 months ago

The command should have been npm update eslint-plugin-prettier, excuse me! I forgot the update part of the it. 😬

asteinhart commented 2 months ago

Apologies @michplunkett, merged this into my branch and the checks failed there. tried to hunt around for these new errors but not getting much, any idea what the fix could be here?


  Resolving repository path: /home/runner/work/data-visualizations/data-visualizations/.
  Running ESlint on changed files...

  Oops! Something went wrong! :(

  ESLint: 9.4.0

  ConfigError: Config (unnamed): Key "rules": Key "curly": structuredClone is not defined
      at rethrowConfigError (/home/runner/work/data-visualizations/data-visualizations/node_modules/@eslint/config-array/dist/cjs/index.cjs:302:8)
      at /home/runner/work/data-visualizations/data-visualizations/node_modules/@eslint/config-array/dist/cjs/index.cjs:1141:5
      at Array.reduce (<anonymous>)
      at FlatConfigArray.getConfigWithStatus (/home/runner/work/data-visualizations/data-visualizations/node_modules/@eslint/config-array/dist/cjs/index.cjs:1134:43)
      at FlatConfigArray.getConfig (/home/runner/work/data-visualizations/data-visualizations/node_modules/@eslint/config-array/dist/cjs/index.cjs:1163:15)
      at /home/runner/work/data-visualizations/data-visualizations/node_modules/eslint/lib/eslint/eslint-helpers.js:548:39
      at Array.forEach (<anonymous>)
      at findFiles (/home/runner/work/data-visualizations/data-visualizations/node_modules/eslint/lib/eslint/eslint-helpers.js:537:11)
      at async ESLint.lintFiles (/home/runner/work/data-visualizations/data-visualizations/node_modules/eslint/lib/eslint/eslint.js:847:27)
      at async Object.execute (/home/runner/work/data-visualizations/data-visualizations/node_modules/eslint/lib/cli.js:500:23)
  reviewdog: parse error: failed to unmarshal rdjson (DiagnosticResult): proto: syntax error (line 1:1): unexpected token 
  Error: Error running eslint.
Error: Process completed with exit code 1.```
michplunkett commented 2 months ago

If I had to guess, it looks like the config file isn't setup correctly. I'd make a new branch (you don't want to build on branches that were previously merged), and fix it there.