nodejs-docs-samples
nodejs-docs-samples copied to clipboard
feat: compute_consume_specific_shared_reservation
Description
Fixes #<ISSUE-NUMBER>
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
- [x] I have followed guidelines from CONTRIBUTING.MD and Samples Style Guide
- [x] Tests pass:
npm test(see Testing) - [x] Lint pass:
npm run lint(see Style) - [ ] These samples need a new API enabled in testing projects to pass (let us know which ones)
- [ ] These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
- [x] This pull request is from a branch created directly off of
GoogleCloudPlatform/nodejs-docs-samples. Not a fork. - [ ] This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
- [ ] This sample adds a new sample directory, and I created GitHub Actions workflow for this sample
- [ ] This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
- [x] Please merge this PR for me once it is approved
Here is the summary of changes.
You are about to add 1 region tag.
- compute/reservations/createInstanceToConsumeSharedReservation.js:20, tag
compute_consume_specific_shared_reservation
This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:
- [ ] Refresh this comment
Hi @iennae, could you please take a look once again on this PR?
cc: @rsamborski
thanks @BigBlackWolf for the ping. Can you check the changes I made to the one file to clarify the comments? I think there is a lot of specific choices embedded in the samples that could use a little more detail and then some obvious comments that distract from the code. For example the instantiates aren't giving more context to the sample consumers. Could you make updates to the other files to match that style?
In general, I know there are other samples like this, but it really feels import with the number of choices that folks have with compute to better understand how to manage that complexity rather than hide it and end up with copied patterns over time with no understanding of the "why".
Thank you!
I appreciate your review @iennae and thanks for adjusting comments, imho they became much more informative! I agree, that it's worth to correct other samples, however it looks like out of the scope of this PR. As I see, the intention was to create compute_consume_specific_shared_reservation. Don't you mind introducing your suggestions in a separate PR?
Heya, the problem is the other issues I raised and this PR has made me realize there is a lot of repetition in these set of samples whoch is going to make them harder to maintain. Do we have an assigned engineering team that will be ok maintaining these?
Unfortunately, I am not aware of those. Do you think changes, that help reduce repetition should be introduced in this PR?
I think refactoring the new sample that is getting added since it has the same duplicate functions could happen in this PR. Do you know if there is an engineering team that will take on maintenance? My primary concern is the folks who have to maintain these. To be clear, I'm nit saying all the existing samples need to get updated but the new one should be refactoring to make this duplication code maintainable.
@iennae I agree that the Compute code samples suffer from a lot of code repetition, which makes the code samples harder to maintain and read.
However, helper functions are not that simple to implement with the way our code sample system works. We have two ways to work with this:
- Have the documentation page pull multiple code samples, for example: helper function to make a disk, helper function to configure networking and main function that actually creates the new instance. This would require more maintenance on the side of the documentation and users wouldn't be able to simply copy-paste a working code sample from a single box.
- Use a system similar to what I implemented for Compute in the Python repository, where helper functions and main samples get auto-merged into a final code sample file with single region tag. The downside of this is more complicated process of introducing new samples and the final code samples in Python tend to be longer than in other languages.
After years of using the Python way of "compiling" code samples, I'm not sure it was the best way of doing things. I hate code repetition, but on the other hand, I want our users, when faced with a single code sample, to have everything available to them in a very approachable way. It wouldn't be good if we, for example, used a helper function in a code samples and later forgot to include it in the documentation next to the code sample.
That said, I'll have a look at the code and comments to address your other suggestions.
CLA check override note: previous executions worked https://github.com/GoogleCloudPlatform/nodejs-docs-samples/pull/3912/checks?check_run_id=34225819577