FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Added admin pointer by dainemawer · Pull Request #199 · WordPress/performance · GitHub

/ performance Public

Added admin pointer - #199

Merged
mitogh merged 15 commits into
release/1.0.0-beta.1from
feature/add-admin-pointer
Mar 4, 2022
ConversationCommits (15)ChecksFiles changed
Merged

Added admin pointer#199
mitogh merged 15 commits into
release/1.0.0-beta.1from
feature/add-admin-pointer

Conversation

dainemawer commented Mar 1, 2022

Copy link
Copy Markdown
Contributor

Summary

Fixes #193

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.

dainemawer added Infrastructure Issues for the overall performance plugin infrastructure Needs Review labels Mar 1, 2022
dainemawer added this to the 1.0.0-beta.1 milestone Mar 1, 2022
dainemawer requested a review from felixarntz as a code owner March 1, 2022 09:02
dainemawer added the [Type] Feature A new feature within an existing module label Mar 1, 2022
jjgrainger self-requested a review March 1, 2022 14:05
jjgrainger previously requested changes Mar 1, 2022
View reviewed changes

jjgrainger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good @dainemawer , just a couple of minor things to address.

Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
mitogh reviewed Mar 1, 2022
View reviewed changes
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
felixarntz requested changes Mar 1, 2022
View reviewed changes

felixarntz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@dainemawer Left a couple of comments. This looks on the right track, but there's a lot of code here that can be simplified, and overall we need to be more careful of not enqueue things too aggressively.

Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
felixarntz changed the base branch from trunk to release/1.0.0-beta.1 March 3, 2022 00:47

felixarntz commented Mar 3, 2022

Copy link
Copy Markdown
Member

@dainemawer FYI Since we've branched off from trunk into a release/1.0.0-beta.1 branch, I've updated the base branch of this PR to that new branch. So going forward, please make sure to work off release/1.0.0-beta.1, and specifically not merge trunk into this again.

dainemawer commented Mar 3, 2022

Copy link
Copy Markdown
Contributor Author

Roger that @felixarntz will fix the merge conflict here!

felixarntz reviewed Mar 3, 2022
View reviewed changes

felixarntz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@dainemawer Left a bit more feedback here, this is looking close now.

Comment thread admin/load.php Outdated
Comment thread admin/load.php
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
mitogh reviewed Mar 3, 2022
View reviewed changes
Comment thread admin/load.php Outdated
Comment thread admin/load.php Outdated
mitogh and others added 9 commits March 4, 2022 12:17
Removal of additional indentation level.
ab67d1e
Create admin url using admin_url function.
bfc9c91 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Update hook and function definition
906dea1 
Make sure the function is only added when need it.
Replace constant with string literal instead.
8725a0c 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Replace constant with string literal instead.
3a55053 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Merge branch 'feature/add-admin-pointer' of github.com:WordPress/perf…
f196b06 
…ormance into feature/add-admin-pointer
Removal of not used constant
33ffd46
Removal of empty space
a4d49a9
Removal of non required concatenation.
a60ebc1 
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
mitogh requested a review from felixarntz March 4, 2022 19:05
felixarntz approved these changes Mar 4, 2022
View reviewed changes

felixarntz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Awesome work @dainemawer!

Thanks @mitogh for the final iteration.

felixarntz dismissed jjgrainger’s stale review March 4, 2022 21:13

Feedback has been addressed.

adamsilverstein reviewed Mar 4, 2022
View reviewed changes
Comment thread admin/load.php Outdated
adamsilverstein approved these changes Mar 4, 2022
View reviewed changes

adamsilverstein left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

one tiny spacing change to align the values, otherwise 👍🏼

Removing non required space.
b3ea66e 
Co-authored-by: Adam Silverstein <adamjs@google.com>
mitogh merged commit fa5c12f into release/1.0.0-beta.1 Mar 4, 2022
mitogh deleted the feature/add-admin-pointer branch March 4, 2022 23:02
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

mitogh mitogh left review comments
adamsilverstein adamsilverstein approved these changes
felixarntz felixarntz approved these changes
jjgrainger Awaiting requested review from jjgrainger

Assignees

No one assigned

Labels

Infrastructure Issues for the overall performance plugin infrastructure [Type] Feature A new feature within an existing module

Projects

None yet

Milestone

1.0.0-beta.1

Development

Successfully merging this pull request may close these issues.

Provide admin pointer informing about the plugin's admin screen

5 participants

Back | FazBrowse Home | New Git URL