-
Notifications
You must be signed in to change notification settings - Fork 409
Add publish template operation #814
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
lahirumaramba
commented
Mar 13, 2020
- Implement publishTemplate()
- Add force publish support
- Add unit tests for publish template
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.
Looks pretty good. Just a couple of suggestions in response to your comments.
.to.throw('ETag must be a non-empty string.'); | ||
}); | ||
}); | ||
|
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.
New test case for null
, undefined
, and empty
etags.
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 with a suggestion.
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 with a suggestion.
'invalid-argument', | ||
'ETag must be a non-empty string.'); | ||
} | ||
const path = validateOnly ? 'remoteConfig?validate_only=true' : 'remoteConfig'; |
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.
const path = validateOnly ? 'remoteConfig?validate_only=true' : 'remoteConfig'; | |
let path = 'remoteConfig'; | |
if (validateOnly) { | |
path += '?validate_only=true'; | |
} |