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.
Problem/Motivation
No changelog entry for 8.3 yet.
Proposed resolution
Add a changelog entry with sufficient items covering the changes. Look for and tag new issues with the 8.3.0 release notes tag.
Remaining tasks
Expand with missing items.
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#76 | 2846830-76.patch | 6.44 KB | cilefen |
#76 | interdiff-75-76.txt | 887 bytes | cilefen |
#75 | interdiff-changelog.txt | 1.47 KB | Gábor Hojtsy |
#75 | 2846830-75.patch | 6.69 KB | Gábor Hojtsy |
#74 | interdiff-changelog.txt | 3.46 KB | Gábor Hojtsy |
Comments
Comment #2
webchickHere's a starter patch. This will definitely need more work, esp. to resolve the @todos.
Comment #3
dagmarThis #1446932: Improve statistics performance by adding a swappable backend seems quite relevant for performance and scalability.
Comment #4
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI just now committed #2796173: Add experimental Field Layout module to allow entity view/form modes to switch between layouts
Comment #5
Gábor HojtsyUpdated with Field Layout module. Also removing extra whitespace that should not be there. I think while this needs work, the more appropriate status may be needs review, since it also need to get stuff identified to add :)
Comment #6
timmillwoodIt seems like the interdiff in #5 is not based on #3 because some items have been removed but aren't displayed in the interdiff.
Comment #7
timmillwoodFixing #6.
Comment #8
Gábor HojtsyWups, sorry I thought I was using the latest patch :/ @timmillwood: are the content moderation/workflow items well represented? Any other major change that should be here (and in the release notes?)
Comment #9
timmillwoodI don't think there are any major changes, but here's a summary of some and you can choose if they're worth adding:
- Moderation now works on non-translatable entity type.
- Moderation can be enabled on entity types without a bundle (but not via the UI).
- EntityPublishedInterface and EntityPublishedTrait have been added and are being used by Node and Comment
- originalRevisionId property has been added to revisionable entities and is populated with previous revision ID when the ID is updated.
- When setting an entity to published it now gets published (as long as the entity implements EntityPublishedInterface) this only used to work for Nodes.
- Better handling of errors when trying to bulk (un)publish moderated entities.
- When reverting to a previous revision the moderation state now gets reverted too.
Comment #11
Gábor Hojtsy@timmillwood: most of those sound like worthy improvements to mention to me. Similar to how we recited such improvements for REST earlier. Eg. mentioning the new published interface would help people adopt it. This one sounds the most obscure, but the others could IMHO be fleshed out a bit to include in the changelog:
Comment #12
Gábor HojtsyComment #13
xjm#1999722: [policy] Define coding standards for anonymous functions (closures) should also go into the coding standards list.
Comment #14
dawehnerSome small improvement.
Comment #16
timmillwoodAdding some of my suggestions from #9 which @Gábor Hojtsy suggested would be good additions in #11.
Comment #17
xjmI'm not sure if 8.3.x will ship with Symfony 3; discuss here: #2712647: Update Symfony components to ~3.2
Comment #18
Gábor Hojtsy@timmillwood: can you also make sure the respective issues are tagged "8.3.0 release notes"? Thanks a lot!
Comment #19
timmillwood@Gábor Hojtsy - Done!
Comment #20
webchickHere's a version that's up to date with the list of issues with the 8.3.0 release notes tag. I also moved some of @timmillwood's suggestions out of authoring experience improvements (which is for user-facing changes) and down to API improvements section.
Also, @dawehner added some things in #14, but apart from #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method I couldn't find corresponding issues for those at a glance. Would you please be able to update those issues with the tag, so that we can keep the release notes in sync?
Comment #21
Wim LeersConfirming this.
The first one is rather confusingly named, so much so, that it's actually wrong.
Second and fourth are accurate.
Third is also confusing.
Improved it.
Comment #22
Wim LeersWTF, d.o ate my files!?Apparently they're there in the "files" table just below the IS, but it's missing from my comment. I think I just found a pretty big bug…Comment #23
webchickWeird. :\ Now my file/interdiff is totally gone from the files table. Let's try this one more time. (Working from Wim's patch.)
Removed the Symfony 3.0 issue since that is only getting committed to 8.4.x, and unified the wording on a couple of the @todo entries with the release notes we're prepping.
Comment #24
webchickSee also #2848192: Add issue #s to CHANGELOG.txt entries for how we might want to format this (later, after we get the text right) to make release notes generation more straight-forward.
Comment #25
xjmCouple notes from editing the release notes draft:
Done now!
(and put it at the end)
Comment #26
Gábor Hojtsy@xjm: Is PHP CodeSniffer and Drupal Coder being added as new development requirements really the least important coding standards change (since you suggest to move it last). It sounds like the biggest coding environment change ever to me. Am I overlooking something?
Comment #27
cilefen CreditAttribution: cilefen commentedIt's not in itself a coding standards change.
Comment #28
Wim LeersI came here to add #2808233: REST 403 responses don't tell the user *why* access is not granted: requires deep Drupal understanding to figure out to the changelog, but it's already in there :)
I also came here to update the CKEditor-related changelog entry, because both #2828494: Update CKEditor library to 4.6 and #2848215: Update CKEditor library to 4.6.2 landed, yay!
Comment #29
xjm@Gábor Hojtsy Yeah the addition of the dev requriements is basically just adding another couple libraries that composer will slurp down if you are composing it for development (but not in packaged releases and not if you follow the instructions to use
--no-dev
for production). They were already the tools our coding standards rules were written for; we just didn't indicate that in our composer.json and so you had to know to install them manually.Comment #30
xjmOr the IS of #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core puts it clearly:
Maybe we could state that in the CHANGELOG to make it clear to people what "add dev dependencies" means here. Something like:
Comment #31
wturrell CreditAttribution: wturrell as a volunteer commented- Add testing improvements #25 xjm item 4
- Move phpcs/coder to bottom of Coding Standards, using xjm's text #30 (verbatim, - typo)
- Mention new Status report.
Comment #32
xjmGreat, thanks @wturrell!
Since it's not a proper noun, I think we should lowercase "status".
This goes over 80 chars on the first line, so we should rewrap it.
I think then the outstanding things are to make a Content Moderation section, and then just adding additional things that go in as they come.
Comment #33
xjm(Updating issue credit.)
Comment #34
wturrell CreditAttribution: wturrell as a volunteer commentedNot entirely clear if we just want to split 'Improved developer APIs' (adding a new Content Moderation API section) or merge the two content mod items there with the non-API ones in 'Improved authoring features'.
Comment #35
xjmI think the next top-level bullet after "Improved site administration experience" could be something like "Content Moderation improvements (experimental)". Then that would include all the authoring, admin, and DX improvements for that module that are currently spread between other categories (as well as #2844594: Default workflow states and transitions which is not added yet since it was just committed yesterday).
We would keep this one where it is under "Added modules", maybe shortening or simplifying it a bit from what it is now:
But then also add under the Content Moderation header:
Thanks, great questions.
Comment #36
wturrell CreditAttribution: wturrell as a volunteer commentedGrateful if somebody could check my summation of #2844594: Default workflow states and transitions.
Comment #37
xjmMarking for review of #36; thanks @wturrell.
Comment #38
xjmThere are numerous specific improvements to the plugin system that unblock contrib to depend on the new layout API; we should add a bullet for those too. @tim.plunkett is working on a CR to summarize them and once that CR is available we can use it to come up with CHANGELOG text. Marking NW for that.
The workflows improvements look good. Let's work on the text for these two:
I think we should describe these changes in a way that is helpful for a less technical audience. @timmillwood, any suggestions on how to improve these?
Comment #39
wturrell CreditAttribution: wturrell as a volunteer commentedProvisionally add #2852636: Update jQuery to version 2.2.4 (intentionally not troubling Testbot with this)
[Edit] On related note, should anyone feels like reviewing #686892: Update JavaScript section of Changelog - these are library updates that apparently got left out of the 8.0 release.
Comment #40
anavarrePer #2824548: Move token info cache to data cache bin and #2824547: Change ViewsData to use the default cache bin instead of discovery.
Comment #41
anavarreAnd back to work per #38
Comment #42
wturrell CreditAttribution: wturrell as a volunteer commentedAdd (provisionally) #2842298: [policy, no patch] Drop IE9 and IE10 support from Drupal 8.4.x.
I felt it unnecessary to mention April 11 specifically (seems that's more about killing off the last OS that can run them.)
Comment #43
xjmGood idea mentioning this. Rather than using a dash I'd start a new sentence for "Microsoft". I'd also remove the clause about it being hard and costly because we don't need to make excuses; just communicate the info.
Thanks @wturrell.
Comment #44
wturrell CreditAttribution: wturrell as a volunteer commentedBrowser support text changes, per #43.
Comment #45
cilefen CreditAttribution: cilefen commentedThere is an "it's" vs "its" typo in that patch which can be fixed in the next addition.
Comment #46
xjm@wturrell, by the way, leaving the issue NW does not actually save anything testbot-wise. All untested patches will be tested when the issue is marked NR, including the old ones. So same number of test runs either way and it is just better to mark the status that makes sense, i.e. NR for the changes. :)
If you do want to prevent testbot from testing something, you should be able to add a
.txt
extension, or I think ending the filename with-do-not-test.patch
might still work.Comment #47
wturrell CreditAttribution: wturrell as a volunteer commentedI've been working through some of the tagged issues…
- Fixed indenting of whole 'Added modules' section
- Fix it's/its typo, #45
- Added Twig and random_compat upgrades
- Mentioned CK Editor includes a new skin
- Added 'Allowed HTML tags' input -> texture
- Mentioned shortened Twig cache directory length + warning for Windows users.
- Mentioned the EntityType collection label and have given "Workflows" as an example.
- Explicitly mentioned the first batch of the WTB to BTB conversion.
- Added a 'Package Management' section (could maybe be elsewhere) with note about use of new module/theme repo endpoint.
@xjm (re: Testbot) - thanks, I should have guessed it would do that… I'm aware of (read: overly obsessive about) the amount of CPU needed to run each job, so like to skip them if the changes make it unnecessary and will hold up the queue/waste energy.
Comment #48
Wim Leers@wturrell: looks good!
I doubt these merit a changelog mention?
Comment #49
xjmThe first Gábor and I agreed to mention since it was actually a compelling usability fix with security implications even because of the user impact, even though small. We can probably make it sound a little more exciting. ;)
The second, not sure, we should check what we said when it was tagged.
The third should not specifically be about those constants. It should instead be mentioned as a general best practices change:
Or something like that.
Comment #50
wturrell CreditAttribution: wturrell as a volunteer commentedThis version has xjm's revised text and placement for items 1 and 3 (which I can't improve on).
Background on item 2 if it helps, (as someone coming to it relatively fresh):
This is the tagged issue #2767025: Add entity type label for a collection of entities. It was spun off from #1850080: Entity type labels lack plurality, cannot generate UI text based on label if plural is needed, and was intended (so says the issue summary) for things like page titles that needed an uppercase plural (e.g. Workflows).
Webchick also cited it as making issues like #2811407: "Workflow entities" admin page title should be "Workflows" considerably easier to fix - i.e. compare the work needed in 2811407-27.patch to change a title from "Moderation state entities" to "Workflows" versus the rather cleaner 2811407-30.patch we were able to commit.
Comment #51
Wim Leers#49: thanks, that all makes a ton of sense :)
Comment #52
Pavan B S CreditAttribution: Pavan B S at Valuebound commentedLine exceeding 80 characters
Comment #53
Pavan B S CreditAttribution: Pavan B S at Valuebound commentedModified one line which contain more than 80 characters. Applying the patch, Please check
Comment #54
xjm@Pavan, thanks for keeping an eye on coding standards. That line was just a todo during development, not part of the intended final patch, so we did not need to worry about it.
Comment #55
wturrell CreditAttribution: wturrell as a volunteer commented- jQuery 2.2.4 confirmed IN (NB: jQuery UI 1.12, which wasn't in changelog, is pushed back to 8.4.x)
- normalize.css 5.0.0 OUT (pushed back to 8.4.x)
- Have left the IE9/10 IN (now 8.4.x), but labelled "Advance notice:" – though more likely you just want to delete...
Comment #56
wturrell CreditAttribution: wturrell as a volunteer commentedAnother refresh:
- Short array syntax is done.
- Clarify what the new anonymous function coding standard actually is.
Comment #57
Gábor HojtsyAdd https://www.drupal.org/node/2513534 :)
Also https://www.drupal.org/node/2846782 should be added here IMHO.
Can we move this to the first item of this sublist? That sounds like the biggest change here honestly.
Add a "Migration improvements (experimental)" section and add https://www.drupal.org/node/2669964 and https://www.drupal.org/node/2225717 (which are tagged for release notes)
This needs https://www.drupal.org/node/2751325 added.
*Maybe* this should be added here, but also maybe in a misc improvements section, but https://www.drupal.org/node/2075889 definitely needs a mention.
Still todo. Maybe "Replaced the deprecated Symfony ExecutionContextInterface by subclassing from ConstraintValidator to prepare for an update to Symfony 3."
Added
This did not land in 8.3.x so should be removed from here. (Based on https://www.drupal.org/node/2572795)
Remove the intro text and keep from "PHP CodeSniffer..." onwards
Are these two the same? Or are they different? How?
"uses the new" IMHO sounds better
Comment #58
xjmI think "Enlargement of JS test coverage." got re-added in a reroll or something; let's remove it again please. :)
Comment #59
naveenvalechaAccommodated the #57 and #58 changes
#57.1 - Done
#57.2 - Done. Moved it to top as its a big UX win.
#57.3 - Done
#57.4 - Done
#57.5 - Pending
#57.6 - Pending
#57.7 - Done
#57.8 - Done. Removed
#57.9 - Done
#57.10 - Pending
#57.11 - Done
#58 - Pending
P.S. Using this issue as an example to explain how to address the feedback and post the rerolled patches.
//Naveen
Comment #60
Gábor Hojtsy@naveenvalecha: I think I needed to review and change all the changes you made. Reasons:
1. You included the issue titles verbatim. Issue titles normally explain the problem. Changelog normally explains what changed, not what the problem was. Eg. "Migrate Drupal 7 core node translations to Drupal 8." (issue title) should become something like "Drupal 7 core node translations are now migrated to Drupal 8." (changelog). Even worse was "All serialized values are strings, should be integers/booleans when appropriate." where what we need to explain is that they are NOT strings anymore, right? So "Serialized boolean and integer values that used to be strings are now booleans and integers."
2. In several cases the changed or added lines exceeded the line limit.
Also resolved the remaining todos, one of which I directly provided suggested text above (and used it verbatim now).
Comment #61
Gábor HojtsyDuh end of line space. May be fixed on commit if no other problems?
Comment #62
jibran#2862254: Update non-Symfony dependencies before 8.3.0 and #2859772: Update Symfony components to ~2.8.18 are missing.
Comment #63
jibranShould we mention coder version in vendor library section?
This is missing #2866109: Composer/installers update to 1.2.0 adding support for installing custom modules and themes to directories other than /vendor
Comment #64
jibranThis should also include #2850797: Prepare our phpunit tests to be BC compatible with phpunit 5.x/6.x imo.
Comment #65
wturrell CreditAttribution: wturrell as a volunteer commentedJust a reminder we still need to decide what to do about #2848192: Add issue #s to CHANGELOG.txt entries
I'm neither strongly pro or anti, but this patch will need to be committed first before anybody attempts it…
Comment #66
xjmThanks @wturrell. @webchick and I agreed to do that for 8.4.x but not for 8.3.x since we're out of time. (Sorry, I forgot to post that on the issue. Will do so now.)
No hyphen in "reuse" (the noun and verb forms are spelled the same).
Need to add the thingy for #2866109: Composer/installers update to 1.2.0 adding support for installing custom modules and themes to directories other than /vendor.
Should we name MoonaLisa here?
s/due/scheduled for/
Also "Microsoft has" would be more idiomatic these days I think, although there's a grammar debate possible there.
I recommend the same change here we made in the FP post. (Pretty sure I also pointed this out in a previous review.) The FP post draft has:
These still don't describe the actual impact of the change. (See #38.)
This is confusing and does not indicate why it matters. I'd suggest:
How is this performance and scalability?
"Various variations" is weird and redundant. :)
"Officially adopted short array syntax and updated all of core accordingly." or something. "Switch" is the wrong tense and implies it wasn't there before, whereas there was already a lot of both syntaxes.
Trailing space here. :)
Missing final period.
Hm, this is more important than "miscellaneous" and we should maybe not have a miscellaneous header for one thing. It's a UX, site builder, and developer improvement.
Comment #67
GrandmaGlassesRopeManI think it's worth mentioning the addition of a
package.json
file and subsequent dependencies for core JavaScript development, which happened here, #2809477: Add ES6 to ES5 build process.---
Something along the lines of, “Added Package.json enabling new JavaScript language features” in Package Management.
Comment #68
xjmNah.
Comment #69
Gábor HojtsyDONE:
----
61/a: added a summary
61/b: added
---
63/1: nope as per @xjm in 68
63/2: done
----
66/1: done
66/2: added in the package manegement section as per Jibran in 63/2
66/3: done, also looked up the actual name Moono-Lisa ;)
66/4: done
66/5: done
66/7: done, I lowercased boolean since I think you did not intend it to be uppercase
66/9: done
66/10: done
66/11: done
66/12: done
-----
67: done
TODO:
66/6: still todo
66/8: still todo; don't know https://www.drupal.org/node/2606772 only mentioned performance once
66/13: still todo; did not find a better place
Comment #70
Gábor HojtsyDuh should not have done what @Jibran suggested in #64 because that is 8.4.x only.
Comment #71
xjmI did mean to capitalize "Boolean" actually; it's a person's name. See on https://en.wikipedia.org/wiki/Boolean:
(We discussed around the time of that issue whether lowercase was acceptable but erred on the side of capitalized IIRC.)
Maybe:
Here (and elsewhere): A few more trailing spaces crept in.
Missing trailing period.
Comment #72
xjmSo I think the answer is "it doesn't really". It is miscategorized in the same way in the release notes draft. It's included because it's one of the handful of non-test-failure criticalish bugs fixed in 8.3.x only. See also: https://www.drupal.org/project/issues/search/drupal?project_issue_follow...
Comment #73
xjmRelated mystery solved: #2809227: Store revision id in originalRevisionId property to use after revision id is updated is a critical, not a DX improvement. So I don't think we should list it in the CHANGELOG, only the release notes. Same for the Twig change (#2606772: Long Twig cache directories can cause failures on some filesystems) and other 8.3.x-only non-regression criticals, which include:
Comment #74
Gábor HojtsyAll right, I don't agree with the Boolean capitalization and neither the removal of mentioning the cue words for routers (I think their inclusion would have helped people figure out what is this even about), but its also 11pm here, so not interested in arguing...
I believe this resolves all of #71, #72 and #73.
What definitely remains is 66/6 and 66/13.
Comment #75
Gábor HojtsyAlso resolved 66/13 after discussion with @xjm.
Comment #76
cilefen CreditAttribution: cilefen commentedThis should cover 66/6.
Comment #77
cilefen CreditAttribution: cilefen commentedI'm going to break protocol a tiny bit so @xjm can commit this and we can release the product.
Comment #80
xjmAlright yeah, I think we've taken this about as far as we can.
Moving this to the correct branch (lol).
Thanks especially to @wturrell for all the help updating this patch over the past couple months, and to @Gábor Hojtsy for painstaking review! Committed to 8.4.x and 8.3.x.
Comment #81
naveenvalecha#60,
Fair enough. That was the quick reroll :)
Thanks for the review!
//Naveen