ENH Add sorting by link type #167
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sabina-talipova
force-pushed
the
pulls/4/link-type-order
branch
2 times, most recently
from
ad747a0 to
84ecd31
Compare
GuySartorelli
requested changes
Jan 11, 2024
There was a problem hiding this comment.
This seems like unnecessary added complexity... why not just change the singular and plural names via private static $singular_name and private static $plural_name?
Edit: Nevermind, that would violate the acceptance criterion "The Link titles remain unchanged everywhere else." - so this is probably the right way to go about it.
GuySartorelli
requested changes
Jan 11, 2024
sabina-talipova
force-pushed
the
pulls/4/link-type-order
branch
2 times, most recently
from
713b256 to
8b2399b
Compare
sabina-talipova
force-pushed
the
pulls/4/link-type-order
branch
from
8b2399b to
912816c
Compare
GuySartorelli
approved these changes
Jan 11, 2024
Comment on lines
+101
to
+103
| uasort($typesList, function ($a, $b) { | ||
| return $a['priority'] - $b['priority']; | ||
| }); |
There was a problem hiding this comment.
Suggested change
| uasort($typesList, function ($a, $b) { | |
| return $a['priority'] - $b['priority']; | |
| }); | |
| uasort($typesList, fn ($a, $b) => $a['priority'] - $b['priority']); |
I won't block merging the PR for this - but in the future we should use the short function syntax for this sort of thing, since we're just returning a value.
The long function syntax is only really valuable when we've got multiple lines of functionality or when the return statement is really long.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Parent issue