Problem/Motivation
Follow-up from #1987726: Convert language content page related callback to new style controller comments #75 and #72
We did an override during a conversion so that the form would be the same, and to keep the conversion in scope and simple, an @todo was added to remove the override and update the tests that expected a particular button name.
Don't forget to enable the module Language
Proposed resolution
Do that in this issue.
Remaining tasks
- (novice) Make a patch that removes the override. git instructions for creating patch | Contributor task documentation for creating a patch
- See what tests fail.
- Make a patch that updates the failing tests.
- Review patch to check it fixes the issue, the change is properly documented and for coding standards. Make sure patch stays within scope of just this issue. | Contributor task documentaiton for reviewing patch
User interface changes
No.
API changes
No.
Comment | File | Size | Author |
---|---|---|---|
#21 | screenshoot_2.png | 143.51 KB | mon_franco |
#21 | screenshoot_1.png | 151.46 KB | mon_franco |
#17 | 2241727.17.patch | 5.07 KB | mon_franco |
#17 | interdiff.2241727.16.17.txt | 752 bytes | mon_franco |
#16 | 2241727.16.patch | 5.08 KB | mon_franco |
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedpostponed on #1987726: Convert language content page related callback to new style controller
Comment #2
mon_franco CreditAttribution: mon_franco commentedComment #3
mon_franco CreditAttribution: mon_franco commentedPatch is uploaded
Comment #4
mon_franco CreditAttribution: mon_franco commentedComment #6
tstoecklerThanks for getting started on the patch!
I have one comment:
This comment should also be removed as part of the patch.
Comment #7
mon_franco CreditAttribution: mon_franco commentedComment #8
mon_franco CreditAttribution: mon_franco commentedI have looked at the next files:
And I have tried to find word "Save" and change it for "Save configuration"
Comment #9
mon_franco CreditAttribution: mon_franco commentedComment #10
mon_franco CreditAttribution: mon_franco commentedComment #12
alimac CreditAttribution: alimac commentedComment #13
mon_franco CreditAttribution: mon_franco commentedReroll. Some tests still fails, but better results now. I will continue tomorrow.
Comment #15
patrickd CreditAttribution: patrickd commentedcorrecting amsterdam tag (No space)
Comment #16
mon_franco CreditAttribution: mon_franco commentedI have changed the file core/modules/path/src/Tests/PathLanguageTest.php because I made a mistake changing two buttons for edit nodes that should not be changed.
Comment #17
mon_franco CreditAttribution: mon_franco commented@YesCT told me next suggestion after review the last patch (#16):
"The todo is saying to remove the override. so the comment will not be needed (when the override is removed)"
So I did it.
Comment #18
tstoecklerAwesomesauce!
Thanks a lot for your persistence @mon_franco!!!
Comment #19
mon_franco CreditAttribution: mon_franco commentedComment #20
mon_franco CreditAttribution: mon_franco commentedWe have found some errors by testing manually the issue
Comment #21
mon_franco CreditAttribution: mon_franco commentedI have tested this issue manually and it works right as you can see on the screen shoots attached
Comment #22
YesCT CreditAttribution: YesCT commentedok.. can't thing of anything that needs work here.
Comment #23
alexpottCommitted 08746e8 and pushed to 8.0.x. Thanks!