-
Notifications
You must be signed in to change notification settings - Fork 295
Only include fee 1.0 extension in nonprod envs #2927
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
Conversation
|
nit: getVisibility() feel free to ignore if there are too many dependencies. |
We need to have this enabled in sandbox, but we wish to wait to enable it for production to make sure that the implementation is correct and that clients can use it. Soon we'll want to do something similar (but the opposite) with the old fee extensions, where we **only** serve them in production (or maybe unit test as well). That will allow us to pass the RST tests that depend on only having the fee extension 1.0.
7eb01c4 to
199e3fc
Compare
gbrodman
left a comment
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.
@gbrodman made 1 comment.
Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @jicelhay).
core/src/main/java/google/registry/model/eppcommon/ProtocolDefinition.java line 110 at r1 (raw file):
Previously, jicelhay (Juan Celhay) wrote…
nit: getVisibility() feel free to ignore if there are too many dependencies.
we're returning a boolean here so i think the best name is actually isVisible
jicelhay
left a comment
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.
@jicelhay partially reviewed 7 files.
Reviewable status: 6 of 7 files reviewed, 1 unresolved discussion (waiting on @gbrodman).
jicelhay
left a comment
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.
@jicelhay reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gbrodman).
|
Previously, gbrodman wrote…
sure! |
We need to have this enabled in sandbox, but we wish to wait to enable it for production to make sure that the implementation is correct and that clients can use it.
Soon we'll want to do something similar (but the opposite) with the old fee extensions, where we only serve them in production (or maybe unit test as well). That will allow us to pass the RST tests that depend on only having the fee extension 1.0.
This change is