Closed (fixed)
Project:
Drupal core
Version:
8.6.x-dev
Component:
javascript
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
25 Jun 2018 at 20:01 UTC
Updated:
31 Aug 2018 at 00:59 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
grandmaglassesropemanComment #3
corbacho commentedComment #4
alexpottDoing this results in a linting issue...
:(
Comment #5
ApacheEx commentedThis patch includes only files which were changed by prettier (#2978964: Use Prettier for formatting core JavaScript).
No lint issues so far :)
Comment #7
ApacheEx commentedI think we need to ignore prettier formatting in this file (
locale_test.es6.js), because some test cases won't work (prettier reformats double quote to single quote or vise versa):Before prettier:
After prettier:
Of course we need to have these test cases - because not everyone uses prettier for custom code :)
So, I've reverted
./core/modules/locale/tests/locale_test.es6.jsand added it to.prettierignoreComment #8
dawehnerMhh, doesn't this effects basically every kind of
Drupal.tstring in core?Comment #9
ApacheEx commentedI've found out,
we have 8 usages which start with
Drupal.t("1) 7 occurrences in
locale_test.es6.js- now, it's ignored by prettier.2) 1 occurrence in
content_types.es6.js- no changes after reformat by prettierso, we are safe.
Comment #10
dawehnerThank you @ApacheEx, that sounds sensible. In the future we can avoid changes, as we use prettier for new things.
Comment #12
ApacheEx commentedRerolled with the latest 8.6.x changes.
Comment #14
ApacheEx commentedForgot to run
yarn run build:js.Should be good now.
Comment #16
xjmThis is the sort of change we generally schedule in advance for during the beta phase of a minor, to minimize disruption to development and keep the minor branches from diverging in a way that adds merge conflicts. If #2978964: Use Prettier for formatting core JavaScript is completed in time to be backported prior to the beta (week of July 30th) then I would recommend scheduling this issue for ~Aug. 6.
Comment #17
ApacheEx commentedHere we go (what was done in this patch):
1. Rerolled with this task: #2978964: Use Prettier for formatting core JavaScript
2. Included #2978964-38: Use Prettier for formatting core JavaScript to avoid failure tests.
3. Applied #2978964-36: Use Prettier for formatting core JavaScript (see 2978964-36-apply-after-prettier.txt) after run prettier.
now eslint rules are passed.
Comment #18
lauriiiI'm planning to commit this on Thursday 9th. Let's try to make sure we have a patch that is committable tomorrow around 2 pm UTC.
Comment #19
xjmHas #2978964-38: Use Prettier for formatting core JavaScript been addressed?
Comment #20
ApacheEx commented@xjm - yes, yes.
I've added
./core/modules/locale/tests/locale_test.es6.jsto the.prettierignoreComment #21
grandmaglassesropeman- deleted node_modules directory
- install dependencies
- yarn run lint:core-js-passing
- no errors reported.
- 🎉
Comment #24
lauriiiConfirmed the same steps and skimmed through the patch quickly to see that it looks correct. Committed to 8.7.x and backported to 8.6.x. Thank you for everyone involved!
Comment #25
corbacho commentedComment #26
xjm