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

Prefer await to then tests #195

Closed

Conversation

brettz9
Copy link
Contributor

@brettz9 brettz9 commented Jul 8, 2020

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • New rule
  • Changes an existing rule
  • Add autofixing to a rule
  • Other, please explain:

What changes did you make? (Give an overview)

  • Linting: Fix as per current prettier
  • Testing: Fix test (was not parsing)
  • Testing: Fix test (missing an error message)

The need for these changes was introduced in 6378bb9

I might suggest enabling Travis on PRs: https://github.com/apps/travis-ci so as to be able to catch test and linting regressions before PRs are merged.

While the linting fix and one of the test fixes were clear as to resolution, in the case of:

-    'a = async () => (try { await something() } catch { somethingElse() })',

+    `a = async () => {
+      try { await something() } catch (error) { somethingElse() }
+    }`,

I had two reasons for my changes:

  1. try cannot be within (), as it requires an expression, and try is a statement.
  2. If you want to use optionally bound catch (i.e., with no (error)), the test would need to be changed to the following (I can change it to this if desired, but didn't want to overcomplicate things if this wasn't needed by the test):
    {
      code: `a = async () => {
      try { await something() } catch { somethingElse() }
    }`,
      parserOptions: {
        ecmaVersion: 2019
      }
    }

I think this should probably be settled before #184 may be assessed (then I can rebase that on this).

@brettz9 brettz9 mentioned this pull request Jul 9, 2020
6 tasks
@brettz9 brettz9 mentioned this pull request Jul 23, 2020
6 tasks
@brettz9
Copy link
Contributor Author

brettz9 commented Feb 11, 2021

Closing as incorporated within merged #184 . Thanks for the merge!

@brettz9 brettz9 closed this Feb 11, 2021
@brettz9 brettz9 deleted the prefer-await-to-then-tests branch February 11, 2021 06:47
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.

None yet

3 participants