- First commit to issue fork.
- 🇬🇧United Kingdom scott_euser
Added changed record here: [#3458241]
On the feedback in #50:
- Typehint added
- I'm not sure how to add params to the interface, it does not support any parameters? Or maybe I misunderstand
- Updated assertions to remove messages
- Status changed to Needs review
10 months ago 9:17am 1 July 2024 - 🇬🇧United Kingdom scott_euser
Hide patches to avoid confusion with MR (and since this type of action tends to get ignored, anyone coming across this, please do not add a new patch, see this doc page for instructions → ).
- Status changed to Needs work
10 months ago 1:04pm 1 July 2024 - 🇺🇸United States smustgrave
Haven't reviewed yet but could issue summary be updated to the standard issue template please
Thank you!
- First commit to issue fork.
- 🇮🇳India shalini_jha
I have checked this issue and issue summary. By calling the newly added toLink() method, I was able to get the link object as expected. I also verified the added test coverage and did not find any additional tags for this issue. I have rebased the MR since it was significantly behind, and the pipeline is now green. Moving this MR for review. Kindly review.
- 🇺🇸United States smustgrave
CR looks like it needs slight tweaking.
Versions seems off
Maybe a before of how users are retrieving this info currentlywith an after of how this is easier.
- 🇮🇳India shalini_jha
Updated the CR according to the feedback based on the issue summary also. Moving this for NR.
let me know if anything else need to update. - 🇷🇺Russia zniki.ru
MR looks good. But should we add test coverage new method at Drupal\Tests\link\Unit\LinkFormatterTest?
- 🇬🇧United Kingdom oily Greater London
RE: #63 I am starting on that.
Outside scope stricly speaking but comment in existing Unit test in Drupal\Tests\link\Unit\LinkFormatterTest states
Tests when LinkItem::getUrl returns a functional URL.
That should be 'functioning' I think. I am creating a (new) similar test with a similar comment. Will change 'functional' to 'functioning'. Could hoover up that slight typo in the existing test.. I mean i think the coder meant 'that works properly' not 'contains the bare necessities but no more'. It is a subtle difference. 'functional' seems correct, but 'functioning' is really what is needed IMO.
- Status changed to RTBC
2 months ago 2:19am 12 February 2025 - First commit to issue fork.
- 🇳🇿New Zealand quietone
I read the IS, comments, the MR and the change record.
I see this is tagged 'Needs change record updates'. That should be resolved before setting an issue to RTBC, thanks.
Continuing to triage, I changed the order of a 'use' statement in the MR. I re-arranged the CR so the most important part, the way things work with this change, is first. I then adding a remaining task to the issue summary. That is an update to the change records, to the tag can remain. I didn't find any unanswered questions. Oh, and I updated credit.
- 🇬🇧United Kingdom scott_euser
Thanks!
- Updated the Change Record to match the BC note from https://www.drupal.org/node/3330231 → - I added in a strong text to separate it from the other strong texts to keep appearing as a separate point.
- I removed that as a remaining task from the issue summary.
- 🇺🇸United States smustgrave
CR looks fine to me.
I did rebase the MR as it was 600+ commits back, after a few re-tries it's still green.
Believe all feedback has been addressed for this one.