Skip to content

Sort modules by the specified criteria on #306 #315

New issue

Have a question about this project? Sign up for a free account to open an issue and contact its maintainers and the community.

By clicking “Sign up for ”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on ? Sign in to your account

Merged
merged 10 commits into from
May 9, 2022

Conversation

mitogh
Copy link
Member

@mitogh mitogh commented Apr 24, 2022

Summary

The introduced changes creates a predictable outcome of modules by using the criteria specified in #306 to order the modules first by focus area, then by experimental and lastly by module name, in this particular case the slug was used as the name due the slug is normalized into a lowercase strings.

Fixes #306

Relevant technical choices

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

The introduced changes creates a predictable outcome of modules by
using the criteria specified in #306 to order the modules first by
focus area, then by experimental and lastly by module name, in this
particular case the slug was used as the name due the slug is normalized
into a lowercase strings.

Fixes #306
@mitoghmitogh added [Type] BugAn existing feature is brokenInfrastructureIssues for the overall performance plugin infrastructurelabels Apr 24, 2022
@mitoghmitogh added this to the First release after 1.0.0 milestone Apr 24, 2022
@mitoghmitogh self-assigned this Apr 24, 2022
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mitogh Looks great, thanks! Just a few minor things worth improving.

@mitoghmitogh requested a review from felixarntz April 26, 2022 03:45
mitogh and others added 2 commits April 26, 2022 18:46
Add warngins back to the terminal in the scenario where a module is
lacking a set of the required properties.

Additionally fixed the fact that the `undefined` check was done against
the object and would result in false values.
Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @mitogh!

@felixarntz
Copy link
Member

@eugene-manuilov Can you please review this so that we can merge it soon?

Copy link
Contributor

@eugene-manuilov eugene-manuilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost looks good to me. Left a comment and a nit-pick suggestion. Cris, please, take a look.

@mitogh
Copy link
Member Author

mitogh commented May 6, 2022

Thanks @eugene-manuilov would address your PR feedback over the weekend.

mitogh and others added 4 commits May 8, 2022 17:43
…:WordPress/performance into fix/306-order-modules-by-custom-criteria
Based on the feedback a single warning is displayed when
multiple problems exists in the module order.
@mitoghmitogh requested a review from eugene-manuilov May 8, 2022 23:00
Copy link
Contributor

@eugene-manuilov eugene-manuilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks, @mitogh. Added one more suggestion that is just a nitpick and shouldn't block this PR. Approved.

Co-authored-by: Eugene Manuilov <[email protected]>
@felixarntz
Copy link
Member

felixarntz commented May 9, 2022

@eugene-manuilov @mitogh Can one of you please address the JS lint error here?

Update: Fixed it below.

@felixarntzfelixarntz merged commit 9134a24 into trunk May 9, 2022
@felixarntzfelixarntz deleted the fix/306-order-modules-by-custom-criteria branch May 9, 2022 18:01
@felixarntzfelixarntz added the no milestonePRs that do not have a defined milestone for releaselabel May 9, 2022
@felixarntzfelixarntz removed this from the 1.1.0 milestone May 9, 2022
Sign up for free to join this conversation on . Already have an account? Sign in to comment
Labels
InfrastructureIssues for the overall performance plugin infrastructureno milestonePRs that do not have a defined milestone for release[Type] BugAn existing feature is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix command line utilities to sort modules correctly
4 participants