-
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
Enhance/#8795 - Implement RRM findMatchedPublication()
action
#9014
Enhance/#8795 - Implement RRM findMatchedPublication()
action
#9014
Conversation
Build files for 46a8673 have been deleted. |
Size Change: +114 B (+0.01%) Total Size: 1.65 MB
ℹ️ View Unchanged
|
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.
Excellent work on this, @hussain-t ! I have left a very small suggestion, after addressing which, we should be able to merge this. Please let me know what you think, thanks!
@@ -19,3 +19,9 @@ | |||
export const MODULES_READER_REVENUE_MANAGER = 'modules/reader-revenue-manager'; | |||
|
|||
export const ERROR_CODE_NON_HTTPS_SITE = 'non_https_site'; | |||
|
|||
export const PUBLICATION_ONBOARDING_STATE = { |
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.
What do you think about changing this to PUBLICATION_ONBOARDING_STATES
(plural)?
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.
Given that only one state can be referenced at a time for a publication, I thought the singular form was appropriate. I can change it if you prefer the plural.
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.
Updated ✅
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, thanks @hussain-t !
Summary
Addresses issue:
findMatchedPublication()
action #8795Relevant technical choices
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