Closed (fixed)
Project:
Override Node Options
Version:
8.x-2.6
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
16 Mar 2022 at 15:17 UTC
Updated:
17 Oct 2023 at 18:04 UTC
Jump to comment: Most recent
Comments
Comment #2
victoria-marina commentedI'll work on this.
Comment #4
victoria-marina commentedKindly review it.
Comment #5
kimberlly_amaral commentedI'll review it.
Comment #6
kimberlly_amaral commentedI checked the changes and ran drupal-check. I found no errors. So I am changing the status to RTBC.
Comment #7
kristen polThanks for reviewing. The module's functionality needs to be tested on Drupal 10. See the following for general instructions:
https://www.drupal.org/community-initiatives/contribution-events-initiat...
Comment #8
kristen polNote I see there is another Drupal 10 compatibility issue here:
#3297795: Automated Drupal 10 compatibility fixes
I did not check if there was overlap between these two issues so that should be done. If they are doing the same work, one should be closed out in favor of the other... ideally, keeping the bot issue.
Comment #9
mmjvb commentedRequesting attention of maintainers as I consider this a no-brainer issue.
It only effect the tests of this module, no changes to the module itself. You might want this for 3.x instead.
On commit, you might want to change
/** @var AccountProxyInterface $user */
to
/** @var \Drupal\Core\Session\AccountProxyInterface $user */
to please drupal-check, assuming it doesn't cause other issues.
EDIT This is the issue why the Automated Fix is now Needs work, consider that out of there and too minor for NW here.
Comment #11
awset commentedas per @mmjvb's comment above, I commit the no-brainer issue and fix the other issue related to the return type of the test method.
Comment #12
mmjvb commented@awset Thank you. Still RTBC.
Comment #13
kristen polI see that most of the changes are in the tests and the only other changes are comments or versions.
I will leave as RTBC but, ideally, this would still be tested on Drupal 10 even though it should work in principle. As such, I'm leaving the manual testing tag. See #7.
Comment #14
mmjvb commentedThat would be testing core rather than this module. Suggest to wait for an RC to do that. Nothing to do with D10 compatibility, any issue should be analyzed as for where the bug is: core or module or both. Consider that the normal bug report procedure. Out of scope for this issue!
Comment #15
roaldnel commentedThanks for the work you have done on this! Do you maybe have a timeline on when this will be merged by chance?
Comment #16
xem8vfdh commentedthanks for the work, look forward to the release :)
Comment #17
kristen polRemoving testing tag since changes are primarily in tests other than info-file changes.
Comment #20
kristen polThanks @opdavies :) Do you want this issue marked Fixed or back to Active in case there are future bot patches?
Comment #21
opdaviesThis has been left open long enough, so I'm marking it as fixed.