-
Notifications
You must be signed in to change notification settings - Fork 595
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
storage: add support for generation on getSignedUrl #1058
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
I signed it! |
CLAs look good, thanks! |
@@ -1173,6 +1173,8 @@ File.prototype.getSignedUrl = function(options, callback) { | |||
}[options.action]; | |||
|
|||
var name = encodeURIComponent(this.name); | |||
var targetGeneration = is.undefined(this.generation) ? | |||
false : encodeURIComponent(this.generation); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Thanks for the pull request, @simonfan! I had a small nitpick on the code and it also appears that the lint task is failing (you can lint the code by running |
@callmehiphop great! I'll have a look on it later today and I'll let you know ;) |
There seems to be some issue with the The error at the travis build was:
|
Hey @simonfan, thanks for this PR! I restarted the test, and that error seems to have disappeared. Travis looks happy now, so back to you, @callmehiphop! |
@simonfan looks great! Would you mind squashing your commits? |
storage: add support for generation on getSignedUrl
Thanks @simonfan! 💃 |
;) |
No description provided.