-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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: support 'sourceGeneration' in 'copy_blob' #4546
Conversation
Hello! Thank you for your contribution. Tests are also needed when you change source code. Also, I thought about this problem. If source_generation was to be put into copy_blob, would other params also go into copy_blob? Also, the other functions such as delete_blob, etc. might need an update as well. I will also continue to think about the problem, but feel free to finish this PR by adding tests. |
I also just noticed that you made the PR using the master branch. A suggestion of using git is to make PRs on a separate branch, so that you can continue to do work and not be worried about automatic updates. If you commit and push on the master branch while doing some other work, this PR will automatically update with this other work. You can use |
Hi, I'm not so expert about git. About PR, what you mean is to make a branch on my repo fork about this feature and do the PR relative to this branch? I'm not an active developer on this repo. Thanks |
@dhermes @tseaver Any comments regarding the other possible kwargs or other functions that could use these possible kwargs? @davidebelloni Yes, git is confusing. I understand that this might be your single pull request, but I just wanted to let you know in case you push to the master branch again, this PR would have automatically update with that commit, which might not be what you want. It's not a big deal for us, but it might make future PRs easier for you to work with. |
Hi @chemelnucfin , Are you waiting for something from me to proceed with this PR? Thanks |
@davidebelloni Hi, I don't see anything that immediately jumps out at me, but since I'm relatively new here I would love to hear @dhermes or @tseaver comments also. Thanks for the PR. |
@davidebelloni Thank you for the patch! |
Hi @tseaver , |
@jonparrott, @frankyn Is there any reason we need to delay a 1.7.0 release? |
go for it |
Closes #4533.