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

chore(java): add a note in README for migrated split repos #1687

Merged
merged 34 commits into from
Nov 7, 2022

Conversation

suztomo
Copy link
Member

@suztomo suztomo commented Oct 31, 2022

It also disables renovate bot and flaky bot for split repositories that have moved to the Java monorepo.

The changes I tested with java-aiplatform (README change) and java-storage (no op) repositories: https://gist.github.com/suztomo/dcea792f5ad2aca03d5b315c46b602a8

Diff as a pull request : googleapis/java-aiplatform#1115

Disable renovate bot and flaky bot for split repositories
that have moved to the Java monorepo.
@suztomo
Copy link
Member Author

suztomo commented Oct 31, 2022

Kokoro failed

@suztomo suztomo added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 31, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 31, 2022
@suztomo
Copy link
Member Author

suztomo commented Oct 31, 2022

======================= 181 passed in 111.57s (0:01:51) ========================
nox > Session test-3.6 was successful.
nox > Running session test-3.10
nox > Session test-3.10 skipped: Python interpreter 3.10 not found.
nox > Ran multiple sessions:
nox > * lint-3.6: failed
nox > * lint-3.10: skipped
nox > * test-3.6: success
nox > * test-3.10: skipped
cleanup

@suztomo suztomo changed the title chore: [java] add readme for migrated split repos chore: [java] add a note in README for migrated split repos Nov 1, 2022
@suztomo
Copy link
Member Author

suztomo commented Nov 1, 2022

Why is yoshi-java not sufficient approvers? It's because this pull request touches synthtool/languages/java.py and this file is not covered by yoshi-java team https://github.com/googleapis/synthtool/blob/master/.github/CODEOWNERS

docker/owlbot/java/README.md Outdated Show resolved Hide resolved
Comment on lines 489 to 490
kwargs["monorepo"] = os.environ.get("MONOREPO") == "true"
kwargs["split_repo"] = os.environ.get("MONOREPO") != "true"
Copy link
Contributor

Choose a reason for hiding this comment

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

Reading environment variables in the logic feels error prone. It feels like this should value should be explicitly provided to this function

Copy link
Member Author

Choose a reason for hiding this comment

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

The challenge is that this java.common_templates() method is called owlbot.py in individual repositories, not the shell script.

To address your concern, I think of providing a keyword argument to java.common_templates() method in owlbot.py:

suztomo@suztomo:~/java-aiplatform$ git diff
diff --git a/owlbot.py b/owlbot.py
index cac43dbb..0dc60a13 100644
--- a/owlbot.py
+++ b/owlbot.py
@@ -27,5 +27,6 @@ java.common_templates(
         ".kokoro/nightly/samples.cfg",
         ".kokoro/presubmit/samples.cfg",
         ".github/CODEOWNERS"
-    ]
+    ],
+    migrated_split_repo=True
 )

This requires me to edit all owlbot.py in ~120 repositories. (feasible but time-consuming)

Do you think of a better way to detect whether the postprocessor is processing monorepo in this Python script?

Copy link
Member Author

Choose a reason for hiding this comment

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

Rather than modifying owlbot.py in ~120 split repositories, passing a special value for 1 monorepo is easy. I'll go for this path.

{
"extends": [
{% if metadata['repo']['library_type'] == 'GAPIC_AUTO'
or (metadata['repo']['repo_short'] and metadata['repo']['repo_short'] in ['java-translate', 'java-dns', 'java-notification', 'java-resourcemanager'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider moving this logic to the template variable collector

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to migrated_split_repo variable in java.py.

docker/owlbot/java/README.md Outdated Show resolved Hide resolved
@suztomo
Copy link
Member Author

suztomo commented Nov 2, 2022

@SurferJeffAtGoogle PTAL

@suztomo suztomo dismissed SurferJeffAtGoogle’s stale review November 7, 2022 03:52

The requested part has been addressed

@suztomo suztomo changed the title chore: [java] add a note in README for migrated split repos chore(java): add a note in README for migrated split repos Nov 7, 2022
@suztomo suztomo merged commit d4b2916 into googleapis:master Nov 7, 2022
@suztomo suztomo deleted the java_monorepo_migration_note branch November 7, 2022 04:06
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.

6 participants