Skip to content

Cleanup II #588

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

Merged
merged 34 commits into from
Mar 1, 2024
Merged

Cleanup II #588

merged 34 commits into from
Mar 1, 2024

Conversation

RickiJay-WMDE
Copy link
Member

@RickiJay-WMDE RickiJay-WMDE commented Feb 26, 2024

NOTE: DELETE test/node_modules/ DIRECTORY AND DOCKER IMAGE wikibase-suite-test-runner-test-runner.

So, to be clear, >95% of this - by lines - is formatting and linting cleanup.

Formatting & linting changes:

  • Formatting .ts, .mjs, .json files - had to add to a few overrides to format JSONs correctly
  • Updated all eslint packages and plugins
  • Somewhere along the way they added a rule about space inside curly brackets
  • Added prettier and prettier-plugin-organize-imports - make formatting consistent, organize imports
  • Added yaml-eslint-parser - formatting and linting YAML files with prettier and eslint

Actual code changes:

  • Every test that was async () => {} is now async function () {}. Ran into an issue that this fixed a few weeks ago - the context is different between the two, so things like this.retries() doesn't actually do anything.
  • Speaking of which, added retries to the flaky special-property.ts test.
  • The lint.sh script was wrong - it was trying to lint twice, because it previously linted both /test and /docs
  • Updated engine to 14.14 - eslint caught that we were using features too advanced for 10, specifically fsPromises
  • Added lint:fix:lint, which just runs eslint, and lint:fix:format, which just runs prettier, and changed lint:fix to run both

@rti rti self-requested a review February 26, 2024 13:46
rti
rti previously approved these changes Feb 26, 2024
@rti rti dismissed their stale review February 26, 2024 17:52

was just a test

@rti rti self-requested a review February 26, 2024 17:52
Copy link
Contributor

@rti rti left a comment

Choose a reason for hiding this comment

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

Is this done by a linting script?

@RickiJay-WMDE
Copy link
Member Author

RickiJay-WMDE commented Feb 27, 2024

Is this done by a linting script?

Yeah - prettier, followed by eslint. Having trouble getting the latter to work on yml files, so I haven't added those to the script in package.json

@RickiJay-WMDE RickiJay-WMDE requested a review from rti February 27, 2024 22:05
@RickiJay-WMDE
Copy link
Member Author

Is this done by a linting script?

Yeah - prettier, followed by eslint. Having trouble getting the latter to work on yml files, so I haven't added those to the script in package.json

Update: Got YAML files working

Copy link
Contributor

@rti rti left a comment

Choose a reason for hiding this comment

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

Shouldn't prettier be installed automatically?

❱ ./lint.sh -f             
Fixing Linting and Formatting issues in Typescript
[+] Building 66.4s (15/15) FINISHED
=> naming to docker.io/library/wikibase-suite-test-runner-test-runner
Need to install the following packages:
prettier@3.2.5
Ok to proceed? (y) 
[error] Cannot find package 'prettier-plugin-organize-imports' imported from /usr/src/test/noop.js

@rti
Copy link
Contributor

rti commented Feb 29, 2024

Question: do we lint from the project root now? Or is this another topic?

@RickiJay-WMDE
Copy link
Member Author

Question: do we lint from the project root now? Or is this another topic?

I think that's another topic - we're still linting from /test here

@RickiJay-WMDE RickiJay-WMDE merged commit 6e4ec28 into main Mar 1, 2024
@RickiJay-WMDE RickiJay-WMDE deleted the cleanup-II branch March 1, 2024 13:18
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.

2 participants