Came here to raise #28. It wouldn't only need a CR, it would need one of those graceful upgrade paths I think, and I'm not even sure precisely how to handle that with an external data source.
Re-adding the RM tag because I think we'd need to consider removal criteria separately and discuss what the upgrade path would be in that situation.
I think it's important that we rely on an external data source as the canonical reference and then provide an API for sites or modules to alter it.
It's an anachronism to refer to roles of framework or product manager for D7 as earlier as these were only defined for D8 and above in 2015. (Also it's weird to list Angie and Gábor as such but not the other past maintainers.) They were described as release managers, core committers, or branch maintainers.
Per DA Engineering only major branches require changes on api.d.o in the current setup.
@lostcarpark, we don't block core on contrib. It's contrib's responsibility to stay up to date with core. :) Drupal 11 has been compatible with PHP 8.1 since 11.1 and the PHP 8.4 pipeline has been been available and passing with without deprecation warnings on core since before then, so it's high time to recommend the rest of the ecosystem follow suit.
We should do this for both actually, since D10 also has no compatibility issues.
- Pipelines updated.
- Notified the DA to update CI templates, API.d.o, etc. →
- Initiated discussion about which alpha modules to remove from the branch (probably only Mailer). This still needs to be done before the issue is marked fixed.
xjm → created an issue. See original summary → .
@voleger, that seems like a very roundabout way to try to get around setting ^ constraints. 😅
Feedback via @alexpott:
OTOH It’s a pretty normal thing to add views to the admin menu that add a page with a business specific listing of content / whatever entity is important to the site. I know that doesn’t need the menu ui listing to do that but when you do it is handy to be able to go somewhere and manage it in the context of all the other links in the menu.
@lauriii suggested a tab for administrative menus.
BTW my proposal in Slack was to title this for the bug, e.g. "Five of seven configurable menus in Standard are admin menus you probably don't want to configure" or such.
My alternate proposal was to provide a grouping label instead of a hiding key, since I am not totally sure about hiding config owned by the site from the administrator.
Edits for clarity, MRs vs. patches, a couple updated links, etc.
@igorgoncalves, to clarify, embedding a screenshot in an issue comment is always good! What @quietone is saying is that you should additionally make sure the Issue Summary is always up-to-date with the latest "before" and "after" screenshots (either embedded or with a link; I prefer embedded myself actually.) It's best to manually put them in the "User Interface" section and hide the auto-generated files links as I've done now. (Those are basically legacy stuff from when patches were a thing, no longer relevant but also not worth removing since the whole issue queue is a sunsetted product as we prepare for GitLab issues.)
Hope that helps!
(Of course, CKEditor integration breaks far more frequently than Drupal majors in general, or that we have control over. So the major version upgrade still isn't necessarily the biggest factor for that hook either. Still, it's worth noting that the hook's data attribute alterations will be completely busted in Drupal 13, silently and with no warning to the site owner.)
To clarify #14, that hook (and all string-pattern-matching hooks) are going to stop working in Drupal 13, which the constraint currently declares Linkit compatible with. So, the data integrity of the module will break, but a site owner won't get any warning when they upgrade to Drupal 13 about the problem. 😀
Just noticed there is also a hook_form_editor_link_dialog_submit(), so my above comment re: being totally data-safe isn't entirely accurate. Always helps to do actual code review. 😀
For what it's worth (again repeating myself from Slack), I think that a reasonably competent module maintainer can also make this choice for themselves. Linkit's primary APIs are filter and formatter plugins, which are not likely to break data even on an API incompatibility. @nicxvan pointed out the hook_form_alter() in the .module file, but that's just as likely to break in a minor as a major, because we break render array structures in minors all the time. (That said, it will need to be updated to new Attribute-based hooks for D12, of course, so the constraint might not be technically accurate right now.)
Maybe we should move this ticket to core for a more general policy discussion since I don't think this is necessarily just a linkit discussion.
These issues already exist somewhere (sorry I actually wasn't able to find them just now but I promise they do!).
@joseph.olstad, I believe you're overlooking an advantage of the current practice, which is that it allows us longer support lifetimes for core versions without needing to change Symfony major version requirements in the middle of a core major version. We did this once in Drupal 8 (upgrading from Symfony 2 to Symfony 3) and it was extraordinarily disruptive. To change now, we would have to sacrifice other things you and other site owners want, like a predictable schedule, or a Drupal 10 support lifetime that extends to at least the release of Drupal 12, or a guaranteed specific EOL date for Drupal 12 in December 2026 rather than an upredictable one that could be in June or August or December, or a guaranteed minimum 4 years of support for every Drupal major version.
The important thing from our perspective is that our maintenance minors land on LTS versions of Symfony, so that these LTS-like versions of Drupal have the increased stability offered by the LTSes of Symfony. So Drupal 10.3 and above are on Symfony 6.4, and Drupal 11.4 and above will be guaranteed to be on Symfony 7.4, and the maintenance minors of Drupal 12 (whatever their specific version numbers) will be on Symfony 8.4.
Drupal 12.0 will release with the latest version of Symfony 8 available at the time of its release. That will probably be Symfony 8.1 if it is released in June or August 2026, and Symfony 8.2 if it released in December 2026. However, the maintenance minors during Drupal 12's LTS phase will be on Symfony 8.4. If Symfony keeps their same release schedule in subsequent years (which we hope, but cannot guarantee), then Drupal 13.0 will be on Symfony 9.1 if it is released in June or August 2028, and Symfony 9.2 if it is released in December 2028, but the maintenance minors of Drupal 12 will be on Symfony 9.4.
We need the 6-month lead time after a new Symfony major release to make core compatible; it's part of why we have the major release schedule we do in the first place. However, adopting as early as we can maximizes the length of time we can support each Drupal major. To change from that, we'd have to shorten the support lifetimes of major versions of Drupal. I'm sure that's not what you want.
For me this issue is "Works as designed".
Quoting myself from Slack:
For what it's worth, some folks (including some core release managers) have been advocating for >= constraints generally because the problem of all the modules ceasing to work every two years is worse than the problem of some APIs not being compatible in the next version of some modules. So this isn't actually wrong.
That said the mismatch between the info file and composer.json is non-ideal of course.
From the Slack history, I also have participation from at least myself, @rodrigoaguilera, @MegaChriz, @vijaycs85, @Chris Darke, @dinarcon, and @igorgoncalves. I think Rachel was also there at the Thursday triage session although things are blurring together in my mind already. 😀
Also oops we should have taken this attendance during the BOF!
@smustgrave asked for feedback on the approach here.
I think the updated scope makes sense (it seems like the new test could move cleanly to contrib with Migrate Drupal without altering the coverage or the history of the migration under test).
That said, honestly I'd appreciate feedback from a Migrate maintainer to confirm that the updated decoupling here matches the goal.
Thanks!
There's a script that autoupdates uses to rename its machine names when moving from contrib to core, so this could be repurposed for Gin's rename. Not sure where it lives but @tedbow or @phenaproxima are the folks to ask.
@fjgarlin and I were discussing the new contribution record system this morning and the limitations thereof for sponsored contributors tracking their work.
I would very much like a table view that shows me a list of issues, sorted by my most recent comment (not the last status update), showing the date of my most recent comment and which orgs I attributed. This seems like an easy extension of the view at e.g. https://new.drupal.org/user/65776/contribution-records?field_is_sa_value=0 with additional fields, but rendered as a table view rather than a list view, and not grouped by project.
Ooh yeah, if we can't override it to be higher, I think that's not good. Thanks!
The commit check script fix has landed BTW, so committers will already be -1ing up our machines by default when we commit stuff.
Reparenting. I've listed it on both metas because we should do this in 12.0.x basically as soon as the branch opens, but we must do this prior to 12.0.0-beta1.
I was debating to myself whether this is something we should do only in a major versus whether it is allowable in a minor. Technically, the change is completely allowable in a minor release, because neither tests nor file locations are public API. So long as the autoloader (and GitLab) can still properly serve the testing framework and its base classes and traits, we could do this in 11.3.
This does make it more disruptive than normal:
It's common to apply patches even on live sites. Patches often contain changes to test code and would by default fail if it is missing (see #33. Therefore as a dependency of this issue, we need an enhancement to the patching mechanism to allow applying patches when test code is missing - see this issue.
For me, the value of unblocking Package Manager is still worth breaking almost every composer-patches in a minor release, and this could be committed before 11.3.0-beta1.
In practice, we are very close to 11.3.0-beta1 anyway (about 3 weeks away), and the opening of 12.0.x is the next thing to happen after that anyway. Making it a major-only change might be nicer for site owners, and I can't think of any practical reason to offer parity on the maintenance minor branch for the change. It's not really something that would need to be part of the continuous upgrade path.
Along the lines of #3, issues should only be created once the scope has been signed off by release managers and when someone is about to actually do the work, and/or when a maintainer or mentor is guiding the work. I.e., we don't create a bunch of empty issues. We create metas, get scoping signoff, and then create child issues one at a time.
I'm not opposed to asking that specific issues be reserved for a subsystem maintainer and/or for novice contributors, although I would do this on a case-by-case basis instead of as a blanket rule, and set a time limit expectation. It's nice to use these as mentoring issues, but we don't want to do so at the expense of the release deadlines, so we also want to make sure they're done well in advance of the beta deadline. Maybe we want to give one month for novices a chance to work on it, or something along those lines?
We could use the friendly green comment templates similar to the ones used at contribution sprints. For reference, those look like:
The language we'd use would of course be different, but that formatting is friendly. The goal is to set clear expectations and balance the needs of the issue versus the goals of mentoring.
Thanks @smustgrave and @quietone!
Committed to 11.x, and cherry-picked to 11.2.x as a patch-safe docs bugfix. Thanks @mstrelan!
This issue is currently doing a lot more than correcting outdated references to https://www.drupal.org/docs/8/theming. 😀 Comments on MR. Thanks everyone!
I am trying to understand how this has been wrong for 11 years.
Is there a way we can validate the whole file?
I checked and both handbook pages have been updated, so I think this can be marked fixed now!
However, maybe we technically need this change to be made in Drupal 12 (not 11) since technically we shouldn't change platform requirements in a minor?
We should add a snippet to the 11.3.0 release notes either way.
Now testDanglingReferencesInAnEntityReferenceFieldFromIssue2968972() is a method name, y'all.
Yay actually following documented typing for strict type enablement potential. Committed to 11.x and cherry-picked to 11.2.x as a patch-safe cleanup to individual tests. Thanks everyone!
Committed to 11.x. I considered backporting it to 11.2.x without the rule with some fancy git tap-dancing, but there are also at least two merge conflicts with the fixed comments, so since this is just basically silly typos I think it's safe to leave this one as 11.x only.
Thanks everyone!
@longwave I'm not sure they've officially agreed on that yet; one of the maintainers is proposing that and hopes to get agreement on it but it might not be 100% confirmed yet.
Tested it by committing 📌 Forbid '@encode' annotation RTBC .
Thanks @joaopauloscho and @nicxvan!
Thanks @danielveza for reporting this. I do agree that using a machine name that references a real existing module and block is misleading.
I disagree though with the proposal of fixing it by using random machine names. We know from long experience that random values in tests are almost always a bad idea. They don't substantively add to test coverage, they make tests harder to read, and they substantively increase the number of random test failures. For example, see my comment on the merge request.
I would be better to come up with fixed test strings for each test block that describe the purpose of that test block.
Thanks!
I went to look for this in the RTBC queue and discovered 📌 Set memory limit for `composer phpcs` command Needs review instead. 😀 Even if that goes in, I still think this change is worth making. (Haven't had a chance to test yet though; will do so when I can unless someone else beats me to it.)
Committed to 11.x, and cherry-picked to 11.2.x as a patch-safe test bug fix. Thanks everyone, and thanks for participating in the DrupalCon Vienna contribution sprint!
Created the followup: 📌 PluginTestBase is not actually a "base class for plugin API unit tests" Active
I think @ranzinator2000 is the d.o username for @manuel-ranzmeir and have updated the contribution record accordingly. Please let me know if this is not correct!
The diff is not applying, so I attempted a rebase through the UI.
I checked the base class and confirmed that it does indeed have secret dependencies on both Node and User. I also confirmed that the three tests updated in the patch are its only descendents. Nice find!
@berdir re: #6, we actually raised some of the same concerns about the architectural entanglement with comment in #3336276-4: [Policy] Deprecate History module → . When it was discussed again 18-some months later at DrupalCon Barcelona there was consensus among the Core Leadership Team to sign off on remove it, but that doesn't mean we're requiring that it be removed before Drupal 12 or anything. So we did take it into consideration, but I can also definitely see it happening in D13 instead of D12.
I was talking about this with @andypost in person at DrupalCon Vienna while reading @berdir's comment.
Discussed with @alexpott. Having this dependency added separately from ✨ Use symfony/runtime for less bespoke bootstrap/compatibility with varied runtime environments Active is a good separation of concerns; however, we don't want to end up in a situation where this dependency is added to a minor release without the functionality it unlocks. Since we're pretty close to alpha1 of 11.3 and we probably don't want to make a change as dramatic as ✨ Use symfony/runtime for less bespoke bootstrap/compatibility with varied runtime environments Active in that release since it's so soon and it would not give us enough time for review and testing, we agreed to postpone this issue on 11.3.x being branched. Then potentially commit it to 11.x after that. I'm adding this issue to 🌱 [meta] Release preparation steps for 11.3.0 and 10.6.0 Postponed to keep track of it.
Thanks!
I got to overhear the reasoning for ✨ Use symfony/runtime for less bespoke bootstrap/compatibility with varied runtime environments Active earlier today and everything here convinces me this is worth the tradeoff of the additional dependency. Thanks @kingdutch!
xjm → changed the visibility of the branch 3549350-remove-useless-code to hidden.
Whenever we remove dead code, it's always a good idea to research the history of the issue to see how it became a dead code in the first place, to make sure we're not accidentally removing the last remnant of an intentional behavior or the last evidence of a bug.
In the case of this issue, I wanted to make sure that removing this condition was in fact a pure cleanup rather than some edgecase that still needed support, so I asked for signoff from either an Ajax subsystem maintainer or a frontend framework manager. @realityloop checked with @tim.plunkett, who researched the history of this issue all the way back to 2013. Tim also reviewed the code side by side with @justafish who is one of the frontend framework managers.
I've also reviewed the code in context myself to confirm that it is indeed dead code since there is nothing in the caller tree to set #typeto ajax. (I'm not totally sure, but I think this may date back to a very old bug with Ajax responses being created within Ajax responses, which is why it got carried forward from very legacy code into modern Ajax response rendering.)
In any case, based on the above signoffs, I'm comfortable committing this to 11.x HEAD.
Thanks everyone for your work on this issue!
I think the minimal scope here of removing the broken link is still a good novice task. We can simply remove the part of the sentence after the comma. The followup issue can handle adding additional content as needed from the historical reference.
I think this is probably not novice since it is not clear what the path forward is. Thanks @arunsahijpal for opening the followup!
We should update the IS here based on the discussion in the original issue.
Discussed this issue with @igorgoncalves. I think that in context the "see below" refers only to the term "key column specifiers", which are indeed documented in detail below. Therefore, as a Drupal core release manager, I am marking this wontfix. Thanks for taking the time to review this and make suggestions!
I updated 🌱 [12.x] [meta] Set Drupal 12 platform and browser requirements at least six months before the release Active to explicitly link the new policy doc that @quietone created. I think that should be sufficient as it will be copied forward for the D13 meta etc.
It is also our policy that the platform requirements are set at least 6 months before the major release date of the major and https://www.drupal.org/about/core/policies/core-change-policies/set-plat... → could be linked wherever that is in the docs (I forget exactly at the moment).
There is one issue that I noticed. It says:
The minimum PHP requirement is set to the highest stable PHP version available at the time beta1 of the next major version is released.
However, that potentially conflicts with the existing requirement that the platform requirements be set at least six months before release. It wouldn't happen with the current release windows that we and PHP have, but suppose that PHP changed their schedule to release a new stable version in February. The old policy would tell us that we couldn't both increase our PHP version requirement and have a June release, but the new policy would indicate that we must increase our PHP version requirement.
It's a hypothetical scenario and I think we would sort it with common sense, but we might want to tweak the language slightly to account for the "six months before" bit, or even add it to the top of the new page since it's existing policy about platform requirements.
benjifisher → credited xjm → .