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

Default eslint config #72

Merged
merged 2 commits into from
Sep 24, 2024
Merged

Default eslint config #72

merged 2 commits into from
Sep 24, 2024

Conversation

stalgiag
Copy link
Contributor

addresses #68

This PR adds a default eslint config. Unfortunately, two of the main rules that should be followed have to be disabled due to abnormal patterns.

  1. empty-block pattern
    These very unique pieces of code rely on an empty block. Since the collection being iterated on is AsyncIterable<any> the most obvious way of avoiding the empty block is not available.

  2. no-unused-var
    There are multiple points where an unused var is being used to support type inference.

It would be easier to add both of these rules back in after a Typescript conversion. For now, empty blocks and unused vars will at least produce warnings. Other rules from the eslint default config are now being enforced.

Copy link
Contributor

@jugglinmike jugglinmike 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. Now that gh-71 is merged, we can move forward on removing those exceptions. Is that a quick patch for you, or would you like me to make a Task to track it?

@@ -94,7 +94,7 @@ export function addTestLogToTestPlan(testPlan, { filepath: testFilepath }) {
*/
export function addTestResultToTestPlan(testPlan, testFilepath, result) {
const test = testPlan.tests.find(({ filepath }) => filepath === testFilepath);
const { testId, presentationNumber, ...resultOutput } = result;
const { testId, ...resultOutput } = result;
Copy link
Contributor

Choose a reason for hiding this comment

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

Yessss

@jugglinmike jugglinmike merged commit 83db9e9 into main Sep 24, 2024
4 checks passed
@jugglinmike jugglinmike deleted the default-eslint-config branch September 24, 2024 00:55
@stalgiag
Copy link
Contributor Author

We would need to do a full Typescript conversion to be able to remove those exceptions. Currently only JSDoc has type awareness because all of the relevant files except the type declarations are .js. A full typescript conversion will be a relatively significant undertaking (~2 days).

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