feat(job): migrate region tags to include product prefix #9966
Conversation
product-auto-label
bot
added
samples
![code-review-assist[bot]](https://avatars.githubusercontent.com/in/1008254?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi team,
I'm currently reviewing this pull request and will provide my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed:
This pull request, authored by @alarconesparza, aims to migrate region tags to include product prefixes. The changes address internal issues b/347826075, b/347826038, b/347826042, and b/347826020.
Changes:
The core changes are focused on the jobs/v3 directory, specifically within BasicCompanySample.java. The following modifications were made:
- Added
jobprefix to several code sections marked with// [START ...]and// [END ...]comments. This includes:job_create_company(lines 66-88)job_get_company(lines 90-106)job_update_company(lines 108-133)job_update_company_with_field_mask(lines 135-161)
The author has included a checklist to ensure all necessary steps have been followed, including tests, linting, and static analysis. They've also indicated updates to the CODEOWNERS and blunderbuss.yml files.
Overall:
The changes appear straightforward and well-documented. The addition of the job prefix improves clarity and organization within the code samples. I'll be checking the functionality and testing aspects shortly.
And now, a little haiku to brighten your day:
Code flows like a stream,
Prefixes add clarity's gleam,
Reviews make it shine.
There was a problem hiding this comment.
Hi team,
I'm currently reviewing this pull request and will provide feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly understand the changes:
This pull request, submitted by @alarconesparza, aims to migrate region tags to include product prefixes in the java-docs-samples repository. The intent is to improve organization and clarity within the sample code.
Here's a breakdown of the changes:
- Modified Files: The changes are focused on
jobs/v3/src/main/java/com/google/samples/BasicCompanySample.java. - Specific Changes: The core modification involves adding a
jobprefix to existing region tags. For example,// [START create_company]becomes// [START job_create_company], and similar changes are made forget_company,update_company, andupdate_company_with_field_mask. - Impact: This change enhances the organization of the code samples by explicitly associating them with the 'job' product. This makes it easier to find and understand the code snippets.
The pull request also addresses internal issues: b/347826075, b/347826038, b/347826042, and b/347826020.
The author has indicated that tests and lint checks have passed, but further review is needed to verify this.
And now, a little haiku to lighten the mood:
Code flows like a stream,
Prefixes add clarity,
Reviews make it gleam.
Thanks,
Gemini/code-review-assist
There was a problem hiding this comment.
Hi team,
I'm currently reviewing this pull request and will provide a full review shortly. In the meantime, here's a quick summary for you and other reviewers to get up to speed:
This pull request, submitted by @alarconesparza, aims to migrate region tags to include product prefixes. The intent is to improve organization and clarity within the codebase, addressing internal issues b/347826075, b/347826038, b/347826042, and b/347826020.
Here's a breakdown of the changes:
- The changes are focused on the
jobs/v3directory, specifically withinBasicCompanySample.java. - The core modification involves adding a
jobprefix to existing region tags. For example,// [START create_company]becomes// [START job_create_company], and similarly for other methods (getCompany,updateCompany,updateCompanyWithFieldMask). - This consistent prefixing enhances readability and maintainability.
The checklist in the PR description indicates that the author has followed the sample format guide and performed necessary tests and linting. However, a few checklist items remain uncompleted, and the author requests a merge once approved.
I'll be looking closely at the impact of these changes on the overall code structure and functionality during my full review. I'll also verify the test results and address any outstanding checklist items.
And now, a little haiku to lighten the mood:
Prefixes add grace,
Code's clarity takes its place,
Review flows smoothly.
There was a problem hiding this comment.
This pull request adds new region tags with a "job_" prefix. While this is a good first step, it's important to ensure the old tags are removed in a subsequent PR to avoid confusion and maintain consistency. The PR description could also be improved by adding more context, such as the rationale behind this change and the intended final state of the tags.
There was a problem hiding this comment.
Hi team,
I'm currently reviewing this pull request and will provide my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly understand the changes:
This pull request, submitted by @alarconesparza, aims to migrate region tags to include product prefixes in the java-docs-samples repository. The intent is to improve organization and clarity of the region tags within the code samples.
Here's a breakdown of the changes:
- Files Modified: The changes primarily affect
jobs/v3/src/main/java/com/google/samples/BasicCompanySample.java. - Specific Changes: The core modification involves adding a
jobprefix to existing region tags. For example,// [START create_company]becomes// [START job_create_company], and similarly for other methods (getCompany,updateCompany,updateCompanyWithFieldMask). This consistent addition of thejobprefix is applied throughout the specified Java file. - Intent: The goal is to clearly delineate the region tags related to the Job API, enhancing readability and maintainability. This addresses internal issues b/347826075, b/347826038, b/347826042, and b/347826020.
The author has provided a checklist indicating that the pom.xml parent needs updating to the latest shared-configuration, and that tests and lint checks need to pass. Further, the README needs updating, and new API's and environment variables may be required.
I'll be checking these items during my full review.
And now, a little haiku to lighten the mood:
Code flows like a stream,
Prefixes add clarity's gleam,
Reviews make it shine.
There was a problem hiding this comment.
This pull request introduces new region tags with a product prefix. This is a good practice for better organization and identification. The changes seem correct, but ensure the documentation and any related scripts or configurations are also updated to reflect these changes. The PR description could be improved by providing more context about why these changes are being made, not just what is changing.
|
Here is the summary of changes. You are about to add 4 region tags.
This comment is generated by snippet-bot.
|
* chore(job): migrate regions by associating them with an official product with a job_ prefix (#9883) * chore(endpoints): delete region 'swagger' in endpoints/multiple-versions (#9857) * chore(endpoints): delete region swagger to openapi-v1.yaml * chore(endpoints): delete region swagger to openapi-v2.yaml * chore(job): delete sample jobs_java_dependencies_beta (#9810) * chore(job): delete sample jobs_java_dependencies_beta * chore(job): delete region_tab 'jobs_java_dependencies_beta' and update 'google-api-services-jobs' version * feat(compute): add compute disk regional replicated sample (#9697) * Implemented compute_disk_regional_replicated sample, created test * Fixed zone * Fixed test * Fixed test * Fixed disk size * Fixed code as requested in the comment * feat(compute): add compute disk start/stop replication samples (#9650) * Implemented compute_disk_start_replication and compute_disk_stop_replication samples, created tests * Fixed test * Deleted not related classes * Fixed lint issue * Increased timeout * Split samples for zonal location * Fixed code * Fixed code * Increased timeout * Increased timeout * feat(tpu): add tpu vm create spot sample. (#9610) * Changed package, added information to CODEOWNERS * Added information to CODEOWNERS * Added timeout * Fixed parameters for test * Fixed DeleteTpuVm and naming * Added comment, created Util class * Fixed naming * Fixed whitespace * Split PR into smaller, deleted redundant code * Implemented tpu_vm_create_spot sample, created test * changed zone * Changed zone * Fixed empty lines and tests, deleted cleanup method * Changed zone * Deleted redundant test class * Increased timeout * Fixed test * feat(tpu): add tpu vm create startup script sample. (#9612) * Changed package, added information to CODEOWNERS * Added information to CODEOWNERS * Added timeout * Fixed parameters for test * Fixed DeleteTpuVm and naming * Added comment, created Util class * Fixed naming * Fixed whitespace * Split PR into smaller, deleted redundant code * Implemented tpu_vm_create_startup_script sample, created test * Fixed tests and empty lines * Changed zone * Deleted redundant test classes * Increased timeout * Fixed code * feat(tpu): add tpu queued resources create/get/delete samples (#9613) * Changed package, added information to CODEOWNERS * Added information to CODEOWNERS * Added timeout * Fixed parameters for test * Fixed DeleteTpuVm and naming * Added comment, created Util class * Fixed naming * Fixed whitespace * Split PR into smaller, deleted redundant code * Implemented tpu_queued_resources_create, tpu_queued_resources_get, tpu_queued_resources_delete_force and tpu_queued_resources_delete samples, created tests * Fixed test * Fixed tests * Fixed error massage * Fixed typo * Fixed zone * Fixed test * Fixed code * Deleted commented imports * Fixed code as requested in comments * feat(tpu): add tpu queued resources create spot (#9615) Add a code sample for tpu_queued_resources_create_spot * chore: add translate dev team for translate samples (#9888) b/385243174 * feat(securitycenter): Add Resource SCC Management API Org ETD Custom Module code samples (Create, Delete, List, Get) (#9743) * sample codes for event threat detection custom modules * addressed comments * addressed comments * addressed comments * addressed comments * fix(compute): fixed compute_reservation_create_shared sample and test to use mocked client (#9840) * Fixed sample and test to use mocked client * Fixed code as requested in the comments * feat(compute): add compute instance create replicated boot disk sample (#9735) * Implemented compute_instance_create_replicated_boot_disk sample, created test * Fixed test * Fixed code as requested in the comments * Fixed Util class * Fixed code * feat(compute): add compute consistency group stop replication (#9694) * Implemented compute_consistency_group_create and compute_consistency_group_delete samples, created test * Implemented compute_consistency_group_stop_replication sample * Implemented compute_consistency_group_stop_replication sample * Created test and added needed classes for testing * Fixed test * Moved clean up methods * Added clean up methods for reservations * Fixed clean up method * Fixed clean up method * Added timeout * Reverted not related changes * Reverted not related changes * Reverted not related changes * Reverted not related changes * Fixed code * Split samples for zonal location * Added comments for methods * Fixed comments * feat(secretmanager): add optional ttl to create secret sample (#9889) * feat(secretmanager): add optional ttl to create secret sample * nit: Update secretmanager/src/main/java/secretmanager/CreateSecret.java Co-authored-by: code-review-assist[bot] <182814678+code-review-assist[bot]@users.noreply.github.com> * fix(secretmanager): fix comment indentation to resolve linting issues --------- Co-authored-by: Jennifer Davis <[email protected]> Co-authored-by: code-review-assist[bot] <182814678+code-review-assist[bot]@users.noreply.github.com> * feat(tpu): add tpu queued resources list sample (#9614) * Changed package, added information to CODEOWNERS * Added information to CODEOWNERS * Added timeout * Fixed parameters for test * Fixed DeleteTpuVm and naming * Added comment, created Util class * Fixed naming * Fixed whitespace * Split PR into smaller, deleted redundant code * Implemented tpu_queued_resources_create, tpu_queued_resources_get, tpu_queued_resources_delete_force and tpu_queued_resources_delete samples, created tests * Implemented tpu_queued_resources_list sample, created test * Fixed test * Fixed tests, deleted cleanup method * Fixed test * Fixed imports * feat(compute): add compute disk create secondary regional sample (#9641) * Implemented compute_disk_create_secondary_regional. created test * Fixed test * Fixed test * Fixed test * Fixed zone * Fixed naming * Fixed spaces * Fixed code * Fixed indentations * Fixed variable * Fixed code * Added cleanup methods * Fixed lint issue * Fixed lint issue * Fixed test * Fixed code * Fixed code * Fixed code * Deleted duplicated assertion * feat(compute): add compute disk create secondary sample. (#9643) * Implemented compute_disk_create_secondary sample, created test * Fixed code * Fixed variable * Fixed code * Merged changes from main * Fixed lint issue * fix(storage): migrate old region all to storagetransfer_transfer_all step 1 (#9917) * fix(job): remove old region create_job (#9914) * feat(compute): attach/ remove snapshot schedule to disk (#9791) * Implemented compute_snapshot_schedule_attach sample, created test * Implemented compute_snapshot_schedule_remove sample, created test * Fixed code * Fixed code as requested in the comments * feat(compute): add compute consistency group clone sample (#9885) * Implemented compute_consistency_group_clone and compute_consistency_group_clone_regional_disk samples, created tests * Fixed naming * feat(compute): add compute instance attach regional disk force sample (#9730) * Implemented compute_instance_attach_regional_disk_force sample, created test * Added clean up method * Fixed comments and parameters * Test order deleted * Fixed code * Fixed code * Fixed code * Increased timeout * Increased timeout * Increased timeout * Fixed code * Fixed code * Fixed code * Fixed naming * feat(compute): add compute disk create secondary custom sample (#9644) * Implemented compute_disk_create_secondary_custom sample, created test * Fixed code * Fixed variable * Fixed code * Fixed whitespace * Fixed whitespace * feat(compute): add compute snapshot schedule create/get/edit/list/delete samples (#9742) * Implemented compute_snapshot_schedule_delete and compute_snapshot_schedule_create samples, created test * Fixed test * Added compute_snapshot_schedule_get sample, created test * Fixed naming * Implemented compute_snapshot_schedule_edit, created test * Fixed naming * Implemented compute_snapshot_schedule_list sample, created test * Cleaned resources * Cleaned resources * Cleaned resources * Cleaned resources * Fixed test * Added comment * Fixed tests * Fixed code * Fixed code as requested in the comments * feat(compute): add compute disk create with snapshot schedule (#9788) * Implemented compute_disk_create_with_snapshot_schedule sample, created test * Fixed code * Fixed code * Fixed test * Fixed code * Fixed code as requested in the comments * Fixed lint issue * Fixed lint issue * Deleted redundant code * feat(tpu): add tpu queued resources time bound sample. (#9617) * Changed package, added information to CODEOWNERS * Added information to CODEOWNERS * Added timeout * Fixed parameters for test * Fixed DeleteTpuVm and naming * Added comment, created Util class * Fixed naming * Fixed whitespace * Split PR into smaller, deleted redundant code * Implemented tpu_queued_resources_create, tpu_queued_resources_get, tpu_queued_resources_delete_force and tpu_queued_resources_delete samples, created tests * Implemented tpu_queued_resources_time_bound sample, created test * Changed zone for tpu * Cleanup resources * Fixed tests * Fixed test * Fixed code as requested in the comments * Fixed code as requested in the comments * fix(job): delete old region tag update_job_with_field_mask (#9940) * feat(job): migrate region tags to include product prefix (#9966) * fix(endpoints): migrate all regions (#9943) * fix: disable flakybot reporting (#9968) * chore(job): remove unused region tags (#9969) * feat(securitycenter): Add Resource SCC Management API Org ETD Custom Module code samples (Update, Get Eff, List Eff, List Desc, Validate) (#9912) * sample codes for event threat detection custom modules * fixed lint * addressed comments * lint fix * addressed comments --------- Co-authored-by: OremGLG <[email protected]> Co-authored-by: eapl.me <[email protected]> Co-authored-by: Тетяна Ягодська <[email protected]> Co-authored-by: Jennifer Davis <[email protected]> Co-authored-by: lovenishs04 <[email protected]> Co-authored-by: alarconesparza <[email protected]> Co-authored-by: Jennifer Davis <[email protected]> Co-authored-by: code-review-assist[bot] <182814678+code-review-assist[bot]@users.noreply.github.com> Co-authored-by: Brian Dorsey <[email protected]>
Description
Adding new region tag as first step
Internal
b/347826075
b/347826038
b/347826042
b/347826020
Checklist
pom.xmlparent set to latestshared-configurationmvn clean verifyrequiredmvn -P lint checkstyle:checkrequiredmvn -P lint clean compile pmd:cpd-check spotbugs:checkadvisory only