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.

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

ci: update template so GAPIC_AUTO repos do not require special approvers for Java code #1494

Merged
merged 5 commits into from
Jul 18, 2022

Conversation

suztomo
Copy link
Member

@suztomo suztomo commented Jul 15, 2022

As discussed with @iennae, with this change the gapic auto repositories will not to have approval entry for Java files in CODEOWNERS. These Java files are auto-generated and thus no point in a special human teams reviewing them.

Also giving java-samples-reviewers Write role so that the last line of CODEOWNERS file is effective samples/**/*.java @googleapis/java-samples-reviewers.

@suztomo suztomo changed the title ci: gapic auto repositories not to have special codeowners ci: gapic auto repositories not to have special approvers for Java code Jul 15, 2022
@suztomo suztomo changed the title ci: gapic auto repositories not to have special approvers for Java code ci: GAPIC_AUTO repos not to have special approvers for Java code Jul 15, 2022
@suztomo suztomo marked this pull request as ready for review July 15, 2022 20:04
@suztomo suztomo requested a review from a team as a code owner July 15, 2022 20:04
@@ -6,7 +6,9 @@
{% if 'codeowner_team' in metadata['repo'] %}
# The {{ metadata['repo']['codeowner_team'] }} is the default owner for changes in this repo
* @googleapis/yoshi-java {{ metadata['repo']['codeowner_team'] }}
{% if 'library_type' in metadata['repo'] and metadata['repo']['library_type'] != 'GAPIC_AUTO' %}
Copy link

@iennae iennae Jul 15, 2022

Choose a reason for hiding this comment

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

suggest adding a comment of something like

# for handwritten libraries keep {{ metadata['repo']['codeowner_team'] }} as owner

Copy link
Member Author

@suztomo suztomo Jul 16, 2022

Choose a reason for hiding this comment

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

Done in the next line.

@iennae iennae changed the title ci: GAPIC_AUTO repos not to have special approvers for Java code ci: update template so GAPIC_AUTO repos do not require special approvers for Java code Jul 15, 2022
@iennae
Copy link

iennae commented Jul 15, 2022

Further clarification from @iennae :
@iennae and @suztomo were discussing the toil that is incurred by teams with autogenerated PRs that require additional approvals. We identified that the source of this issue is arising due to the lack of yoshi team being present for the *.java files.

This PR allows handwritten libraries to keep the review requirements. We didn't access whether handwritten libraries needed sole access over the **/*.java files. It might be the case that it's sufficient to just remove the line in the template
**/*.java {{ metadata['repo']['codeowner_team'] }}

@suztomo
Copy link
Member Author

suztomo commented Jul 18, 2022

To confirm the sample account.

Screen Shot 2022-07-18 at 12 06 58 PM

It turned out that the account exists but has been missing the "Write" role in many repositories. Discussed with Kristen and Averi, I'm granting the role to the team across the repositories.

@suztomo suztomo added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 18, 2022
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 18, 2022
Comment on lines +61 to +62
- team: java-samples-reviewers
permission: push
Copy link
Member Author

@suztomo suztomo Jul 18, 2022

Choose a reason for hiding this comment

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

FYI: @kolea2 @averikitsch This should give "Write" role to the team.

Copy link
Member Author

Choose a reason for hiding this comment

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

@suztomo suztomo merged commit da89e53 into googleapis:master Jul 18, 2022
@suztomo suztomo deleted the gapic_auto_approver branch July 18, 2022 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants