Problem/Motivation

We use prettier to format core JavaScript, but we are several versions behind.

Steps to reproduce

Proposed resolution

Update prettier and reformat all JS.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#9 3607173-nr-bot_tewe4_xa.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3607173

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

longwave created an issue. See original summary.

longwave opened merge request !16186

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review

Upgraded prettier, fixed an incorrect path in .prettierignore, and ran yarn prettier to reformat all code.

We should likely backport this to 11.x for easier cherry-picking of default_admin changes.

godotislate’s picture

godotislate
he/him
commented

Update to prettier has some good but also strange preferences on how to break lines, but this generally looks good.

There's one file that got unminified, so asked a question on the MR about that.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

I wonder where that file came from originally. I think eslint ignores the migrated JS in default_admin, unsure if we should do the same here.

Regarding the unminification it at least makes it editable if we need to in the future, and JS aggregation will re-minify it again via Peast.

godotislate’s picture

godotislate
he/him
commented
Status: Needs review » Reviewed & tested by the community

Sounds good. It probably makes sense to look into that minified JS file in a follow up issue, probably a part of the default_admin clean up effort, if it doesn't already exist. I also noticed there are still a lot of gin references. But all that doesn't affect anything here, and the MR lgtm.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work

Actually it's not that readable so I'm going to skip it via .prettierignore, the default_admin cleanup work can sort it out.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Reviewed & tested by the community

Reverted scroll_sync.js.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3607173-nr-bot_tewe4_xa.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

quietone made their first commit to this issue’s fork.

quietone’s picture

quietone commented
Status: Needs work » Needs review

Rebased

godotislate’s picture

godotislate
he/him
commented
Status: Needs review » Reviewed & tested by the community

Looks good again.