- Issue created by @scott_euser
- Issue was unassigned.
- Status changed to Needs review
8 months ago 11:13am 25 June 2024 - π¬π§United Kingdom scott_euser
Here is the change record from Core https://www.drupal.org/node/3436275 β
- π¦πΊAustralia jannakha Brisbane!
@scott_euser - patch doesn't apply to 8.x-3.4 - should I test on dev branch?
- Status changed to RTBC
7 months ago 10:44am 26 June 2024 - π¦πΊAustralia jannakha Brisbane!
tested MR!58 on D10.3, php 8.3, on 3.x-dev@dev
looks good!Thanks for your fixes!
- π¬π§United Kingdom scott_euser
Great thanks! Yep MRs will always target dev branch of a module. Usually that ends up being fine for patches to the release but yeah often leads to issues. Definitely does need to keep like this as the target for the merge though otherwise it does not help the maintainers.
In any case thanks for reviewing. Over to maintainers then!
- Status changed to Fixed
6 months ago 8:26pm 2 August 2024 - π¬π§United Kingdom scott_euser
Probably easiest is to set:
core_version_requirement: ^10.3 || ^11
To prevent upgrade to latest field group when leaving drupal core outdated.
- πΊπΈUnited States caesius
^ Not recommended on 3.x; see comment on other issue.
- Merge request !62Reapply-fix-deprecations with composer.json/core_version_requirement β (Closed) created by scott_euser
- Status changed to Needs review
6 months ago 6:36am 3 August 2024 - π¬π§United Kingdom scott_euser
Okay updated 2nd branch to have both composer.json and core_version_requirement constraints. Now as long as there are two tagged releases, composer.json will negotiate to not allow update to the latest if not 10.3+. Of course tests fail on 3456987-reapply-change-step-1 as its back to having the deprecated code with the commit revert.
Steps for maintainer:
- Merge 3456987-reapply-change-step-1
- Tag & release
- Merge 3456987-reapply-change-step-2
- Tag & release
- π¬π§United Kingdom scott_euser
So essentially then users will get the reverted version 8.x-3.6 (3456987-reapply-change-step-1) if they are locking themselves in to Drupal 10.2 and not doing a full composer update. Composer won't then let them get 8.x-3.7.
Then those doing a full composer update (so also 10.3) will get 8.x-3.7 (3456987-reapply-change-step-2) because composer will allow it.
- πΊπΈUnited States caesius
This change cannot be applied to Field Group 3.x. This module is covered by the security advisory policy, which I haven't thoroughly read, but I assume it implies that any security updates to a module must be applicable to all supported versions of Drupal. Preventing Drupal <10.3 from receiving new updates to the module would inherently prevent any potential security releases from being applied, which would be a huge problem. Drupal 10.2 is supported until the release of 10.4, so probably until December.
As mentioned, a new full release of this module is required for this code update. BC incompatible changes usually imply a new major version, though some like Linkit β opt for maintaining separate minor versions, i.e. 6.0.x and 6.1.x. However, Field Group is still using 8.x-3.x with no capability for distinguishing minor and patch versions, so a 4.0.0 release is the only option.
- π¬π§United Kingdom scott_euser
Removing deprecated code has nothing to do with security. Does not make any sense to request maintainers maintain multiple versions in order for some sites to stay on old version of core.
- π¬π§United Kingdom scott_euser
Seems sensible options are:
- Wait for December 9th when 10.2 is official also end of security support (end of support was over a month ago) and accept that tests will fail.
- Set composer requirements as per MR 2 to stop update to incompatible version of field group if actively deciding to stay on old version of core.
- π¬π§United Kingdom scott_euser
Ah I see sorry you aren't saying it's a security issue it itself, you are speaking in the hypothetical. If there is an urgent security release for 10
2 core for field group before December 9th the maintainers can of course add back in the deprecated code and make a release. - πΊπΈUnited States caesius
So you're suggesting that they release a 3.7 version that can't be installed on Drupal 10.2, and then potentially later release a 3.8 with a security fix and rollback of this code that can be installed on 10.2, and then release 3.9 with the code re-removed, composer constraint re-added, and the security fix in place?
I think you're missing the point of semantic versioning. Field Group 8.x-3.x has 200,000 installs and is so frequently used that issues for it are sometimes filed to core π Field Groups marked as required are missing red asterisk Needs work . As such, the community needs to be confident that it is stable, and it needs to be available for all supported versions of Drupal, potentially even in-development versions such as Drupal 12.
If this code is absolutely required for automated tests to pass then it should be added to the project, but it can't be done in a way that introduces uncertainty about which version of the module is actually compatible with which version of Drupal. I don't understand the reluctance and obstinance of module maintainers to actually release a new major version of a module when making backward incompatible updates or potentially breaking changes. If any module needs to set an example for how to properly maintain versions and apply semantic versioning, it's this one. "It's a hassle" is not an excuse to do otherwise.
- π¬π§United Kingdom scott_euser
Fair points. Just trying to be considerate to maintainers time, but really I have no idea, maybe it's paid time rather than volunteer. Was ultimately just trying to be helpful to get tests passing for them again but obviously tests aren't covering old versions as became apparent here, sorry for all the hassle this caused!
- π¬π§United Kingdom scott_euser
Just looked back at the background of why I tried to fix the tests, was trying to help get π HTML5 Validation Prevents Submission in Tabs Postponed: needs info 6 year old issue over the line.
- Status changed to Needs work
6 months ago 5:06am 5 August 2024 - π¦πΊAustralia acbramley
My 2c would be to create a new major (4.x, without the 8.x-), there you would set the minimum version to 10.3 and 11. That way phpstan and tests will run on the latest minor and next major and we can keep 8.x-3.x for just security updates.
- π¦πΊAustralia acbramley
acbramley β changed the visibility of the branch 3456987-revert-change-step-1 to hidden.
- π¦πΊAustralia acbramley
In the meantime you could add a baseline to allow phpstan to pass again :) https://git.drupalcode.org/project/field_group/-/merge_requests/65
- Status changed to Needs review
6 months ago 5:16am 5 August 2024 - π¬π§United Kingdom scott_euser
Ignoring it is an option, just need to make sure there are adequate reminders in place. Happy to set a reminder myself to follow up in this issue Jan 2025 if that's the decision.
- π©πͺGermany Anybody Porta Westfalica
@acbramley and @scott_euser if we're all fine with that, we can surely create a 4.x branch as suggested in #23 and merge MR!65
@scott_euser could you create a follow-up for 4.x to only support Drupal ^10.3 || ^11 and remove that ignore?
The last question is, if we should already switch to 4.x now or fix other issues in 8.x-3.x before as I don't think that branch will get no more features then...
- π©πͺGermany Anybody Porta Westfalica
Follow-up: π Fix Drupal 11 deprecations Needs work
- Status changed to Fixed
5 months ago 7:24am 8 September 2024 Automatically closed - issue fixed for 2 weeks with no activity.