Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
See #2915784: 1/3 JS codestyle: camelcase for more information.
This issue is just the changes to color
module, including the BC layers.
Comment | File | Size | Author |
---|---|---|---|
#20 | 2927228-20.patch | 6.42 KB | mikejw |
#16 | 2927228-16.patch | 6.53 KB | mikejw |
#10 | interdiff-2927228-7-10.txt | 420 bytes | GrandmaGlassesRopeMan |
#10 | 2927228-10.patch | 6.29 KB | GrandmaGlassesRopeMan |
#7 | 2927228-7.patch | 6.57 KB | GrandmaGlassesRopeMan |
Comments
Comment #2
GrandmaGlassesRopeManComment #3
GrandmaGlassesRopeManComment #4
dawehnerLooking at the code again the shift_color method is never exposed, as its just defined in the scope of the attach method. Given that I don't think we need a BC layer.
Comment #5
dawehnerI would argue: Let's make it simpler, as its not needed. I would though also argue: Let's not merge it into another issue, as this is a hard problem to think about.
Comment #6
GrandmaGlassesRopeMan- Removes the BC layer. Scope is hard,
shift_color
was only available within the scope ofDrupal.behaviors.color.attach
.Comment #7
GrandmaGlassesRopeMan- returns
.eslintrc.passing.json
to it's original state.Comment #8
dawehnerThis looks like you did not ran the build step :(
Comment #9
GrandmaGlassesRopeManI've fallen victim to my own process.
Comment #10
GrandmaGlassesRopeMan- Actually includes the transpiled code.
Comment #11
dawehnerThank you @drpal!
Comment #12
webchickI got a bit confused by the issue summary which says this includes colour module fixes + BC layers, but no BC layers. :D I see this was removed per #4. When I asked @dawehner for more info on Slack, he pointed out that the shift_color() function isn't accessible from outside, so therefore the BC layer isn't needed. (See https://jsfiddle.net/dc4jpL26/)
Okie doke then!
Committed and pushed to 8.5.x. Thanks!
Comment #15
xjmHad to revert as mentioned in the other issue; please reroll and this can probably go back in with some quick re-review and testing. Thanks!
Comment #16
mikejw CreditAttribution: mikejw at University of Adelaide commentedHere be the reroll. Similar to 1/3 wasn't a straight reroll.
Comment #17
mikejw CreditAttribution: mikejw at University of Adelaide commentedComment #18
dawehnerThank you for your patch @mikejw
Comment #19
webchickTrying to commit this, but https://github.com/alexpott/d8githooks is giving me:
Any ideas?
Comment #20
mikejw CreditAttribution: mikejw at University of Adelaide commentedYeah, needed to run build:js over it ;)
Comment #21
mikejw CreditAttribution: mikejw at University of Adelaide commentedComment #22
dawehnerThank you!
Comment #23
webchickOkay, take two! :)
Committed and pushed to 8.5.x. Thanks!