Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(job): remove old region create_job #9914

Merged
merged 1 commit into from
Dec 29, 2024
Merged

fix(job): remove old region create_job #9914

merged 1 commit into from
Dec 29, 2024

Conversation

OremGLG
Copy link
Contributor

@OremGLG OremGLG commented Dec 24, 2024

Description

Delete old region tag create_job

Fixes
b/347826077

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

  • I have followed Sample Format Guide
  • pom.xml parent set to latest shared-configuration
  • Appropriate changes to README are included in PR
  • 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)
  • Tests pass: mvn clean verify required
  • Lint passes: mvn -P lint checkstyle:check required
  • Static Analysis: mvn -P lint clean compile pmd:cpd-check spotbugs:check advisory only
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners 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
  • Please merge this PR for me once it is approved

@OremGLG OremGLG requested review from yoshi-approver and a team as code owners December 24, 2024 20:06
code-review-assist[bot]
code-review-assist bot reviewed Dec 24, 2024
View reviewed changes
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi team,

I'm currently reviewing this pull request and will have a full review ready in a few minutes. In the meantime, here's a quick summary to get everyone up to speed:

This pull request, submitted by OremGLG, aims to remove outdated code related to region tags in the create_job function within the java-docs-samples repository. The specific changes are:

  • Removal of obsolete region tag comments: The pull request removes the // [START create_job] and // [END create_job] comments from BasicJobSample.java. These comments appear to be remnants of older documentation or examples and are no longer relevant. The changes are localized to lines 76 and 96 of BasicJobSample.java.

The intent is to clean up the codebase and improve readability by removing unnecessary and potentially confusing comments. This addresses issue b/347826077.

Here's a little haiku to lighten the mood:

Old code, now removed,
Clean lines sing a joyful song,
Code's beauty shines through.

Looking forward to discussing this further!

Best,
Gemini

@snippet-bot Snippet Bot
Copy link

snippet-bot bot commented Dec 24, 2024

Here is the summary of changes.

You are about to delete 1 region tag.

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

@product-auto-label product-auto-label bot added samples Issues that are directly related to samples. api: jobs Issues related to the Cloud Talent Solution Job Search API. labels Dec 24, 2024
code-review-assist[bot]
code-review-assist bot reviewed Dec 24, 2024
View reviewed changes
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request removes the redundant create_job tag in BasicJobSample.java, which improves clarity and organization. The PR description is well-written and the checklist is thorough. While the change is small, running existing tests to ensure no regressions is recommended. I've referenced the Google Java Style Guide in this review.

Summary of Style Guide elements referenced:

  • Javadoc comments: Ensuring Javadoc comments are clear and concise is important for maintainability. This was considered when suggesting improvements to the Javadoc for the createJob method.
  • Comment Style: The Google Java Style Guide recommends using // for single-line comments, which is followed in this PR.

jobs/v3/src/main/java/com/google/samples/BasicJobSample.java
@@ -73,7 +73,6 @@ public static Job generateJobWithRequiredFields(String companyName) {
// [END job_basic_job]

// [START job_create_job]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Since the create_job tag is being removed, it's a good opportunity to review the remaining job_create_job tag. Is this tag still necessary, or could it be combined with the job_basic_job tag or replaced with a more descriptive comment explaining the purpose of this section of code? Per the Google Java Style Guide, comments should be clear and concise, providing context and explanation where needed.

jobs/v3/src/main/java/com/google/samples/BasicJobSample.java
@@ -73,7 +73,6 @@ public static Job generateJobWithRequiredFields(String companyName) {
// [END job_basic_job]

// [START job_create_job]
// [START create_job]

/** Create a job. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

The Javadoc comment here could be improved by adding more detail about the function's purpose, parameters, return values, and any exceptions it might throw. Following the Google Java Style Guide, the Javadoc should be comprehensive and informative. For example:

/**
 * Creates a job with the specified parameters.
 *
 * @param jobToBeCreated The job object containing the details of the job to be created.
 * @return The created job object.
 * @throws IOException If an I/O error occurs during job creation.
 */

@iennae iennae merged commit c917ab6 into GoogleCloudPlatform:main Dec 29, 2024
7 checks passed
@OremGLG OremGLG deleted the garciadiaz-migrate-basic-job-step2-347826077 branch December 30, 2024 16:35
ludoch added a commit that referenced this pull request Jan 9, 2025
@ludoch @OremGLG
@eapl-gemugami @TetyanaYahodska @iennae @lovenishs04 @alarconesparza @code-review-assist @briandorsey
Sync with main. (#9972)
dc94374 
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@code-review-assist code-review-assist[bot] code-review-assist[bot] left review comments

@iennae iennae iennae approved these changes

@yoshi-approver yoshi-approver Awaiting requested review from yoshi-approver yoshi-approver is a code owner

Assignees

@grayside grayside

Labels
api: jobs Issues related to the Cloud Talent Solution Job Search API. samples Issues that are directly related to samples.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@OremGLG @iennae @grayside