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

Refactor lib/__tests__/needlessDisables.test.js #5287

Merged
merged 2 commits into from May 12, 2021

Conversation

ybiquitous
Copy link
Member

@ybiquitous ybiquitous commented May 12, 2021

  • Reduce callbacks as much as possible via async/await syntax. (see also https://jestjs.io/docs/asynchronous)
  • Replace the use of stripIndent() with trim(). (avoiding indentation in template string literals)
  • Reindent template string literals for consistency.

Which issue, if any, is this issue related to?

A part of #4881

Is there anything in the PR that needs further explanation?

It's easier to review the diffs if using "Hide whitespace changes".

- Reduce callbacks as much as possible via `async/await` syntax.
  (see also <https://jestjs.io/docs/asynchronous>)
- Replace the use of `stripIndent()` with `trim()`.
  (avoiding indentation in template string literals)
Copy link
Member

@jeddy3 jeddy3 left a comment

Choose a reason for hiding this comment

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

Replace the use of stripIndent() with trim(). (avoiding indentation in template string literals)

We use stripIndent because it, arguably, makes the source files easier to read/scan as the indentation is consistent across the file. I'm in a favour of keeping it and fixing the alignment so that the template literals are aligned with their surrounding JavaScript code. Or are there reasons to remove it? We'll want to apply which ever approach consistently across the codebase.

@ybiquitous
Copy link
Member Author

@jeddy3 Thanks for the feedback! I have no strong opinion and want to keep consistency, so I'll follow our community decision.

Unfortunately, auto-formatting by Prettier is not available, but I’ll re-indent template string literals with TABs.

},
{
line: 11,
column: 21,
Copy link
Member Author

Choose a reason for hiding this comment

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

[note] This column change from 22 to 21 is caused by re-indentation with TABs.

@jeddy3 jeddy3 merged commit db56ce7 into master May 12, 2021
@jeddy3 jeddy3 deleted the refactor-needlessDisables.test.js branch May 12, 2021 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants