-
Notifications
You must be signed in to change notification settings - Fork 291
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix link for RRM module homepage. #9165
Conversation
Build files for a1b6893 have been deleted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's update the URL here, as discussed on our engineering call.
@@ -240,7 +240,7 @@ protected function setup_info() { | |||
'name' => _x( 'Reader Revenue Manager', 'Service name', 'google-site-kit' ), | |||
'description' => __( 'Reader Revenue Manager helps publishers grow, retain, and engage their audiences, creating new revenue opportunities', 'google-site-kit' ), | |||
'order' => 5, | |||
'homepage' => __( 'https://readerrevenue.withgoogle.com/', 'google-site-kit' ), | |||
'homepage' => __( 'https://publishercenter.google.com', 'google-site-kit' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this URL localised? 🤔
We can't expect (community) localisers to know the right URL to send users to, so let's remove this.
'homepage' => __( 'https://publishercenter.google.com', 'google-site-kit' ), | |
'homepage' => 'https://publishercenter.google.com', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually existent in all modules. Should we maybe create an issue to remove all of them as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is a very small change, I've made this myself. I've also opened #9175 to do this for other modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! ✅
Over to you @tofumatt to approve the PR.
Summary
Addresses issue:
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist