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: Generate between split repos and main #8442

Merged
merged 22 commits into from
Sep 26, 2022
Merged

chore: Generate between split repos and main #8442

merged 22 commits into from
Sep 26, 2022

Conversation

lqiu96
Copy link
Contributor

@lqiu96 lqiu96 commented Sep 23, 2022

Following Tomo's script advice:

#!/bin/bash

set -e

modules=$(mvn help:evaluate -Dexpression=project.modules | grep '<.*>.*</.*>' | grep 'java' | sed -e 's/<.*>\(.*\)<\/.*>/\1/g')

for module in $modules; do
  echo "Running for ${module}"
  rm -rf "${module}"
  git clone "https://github.com/googleapis/${module}"
  rm -rf "${module}/.git"
  echo "Done running for ${module}"
done

./generation/delete_non_generated_samples.sh

echo "Done running script"

For now:

  • Ignore pom.xml (assuming they are renovate PRs) -- Can create PR to check only pom.xmls
  • Ignore CHANGELOG.md (from release-please) -- Might need to copy over from split-repos
  • Ignore README.md -- Can copy over from split repos
  • Ignore split repos renovate.json configs
  • Ignore samples/*
    git diff --name-only origin/main | grep -v "pom.xml" | grep -v "CHANGELOG.md" | grep -v "README.md" | grep -v "renovate.json" | grep -v "versions.txt" | grep -v "samples/*" > diff.txt

Probably should ignore owlbot files and .github/*

Shows only the name of the files that have changed: diff.txt

Github has issue displaying the diffs. Might need to only add the files listed in the diff.txt

@suztomo
Copy link
Member

suztomo commented Sep 23, 2022

How about limiting first 1000 diff files in the pull request? (We can't see thousands of files anyway)

Once the script is ready and merged to main, we want a GitHub Actions job that updates a branch periodically. We monitor a pull request from the branch to observe the changes.

@suztomo
Copy link
Member

suztomo commented Sep 23, 2022

This PR reveals Java code diff (maybe manually-maintained tests?). Great.

Screen Shot 2022-09-23 at 11 22 41 AM

@lqiu96
Copy link
Contributor Author

lqiu96 commented Sep 23, 2022

How about limiting first 1000 diff files in the pull request? (We can't see thousands of files anyway)

I didn't want to manually limit/exclude certain files. I've taken a look at a few of the diffs locally and I think quite a few of them can be ignored.

Ignoring these files should help us limit the number of diffs we need to check:

  • CHANGELOG.md (from release-please) -- Might need to copy over from split-repos
  • README.md -- Can copy over from split repos
  • renovate.json configs in each split repos
  • Ignore samples/* -- As we are no longer maintaining them and can look into them later
  • Ignore owlbot.yamls

Need to check:

  • The .java files (either manually or from owlbot)

The pom.xml files shouldn't have custom pom.xml, but I can do a separate and specific diff (either in a new PR or a seperate branch) to check each pom.xml. What do you think about the proposed ignored files above? @suztomo

Once the script is ready and merged to main, we want a GitHub Actions job that updates a branch periodically. We monitor a pull request from the branch to observe the changes.

I can look into a GH action once I flush out the script with the diffs that I want to show.

@lqiu96
Copy link
Contributor Author

lqiu96 commented Sep 23, 2022

Plan forward is two branches

  1. Java only code (from owlbot changes and samples)
  2. Non-Java code

Script will create two branches and create PRs for each branch that show the diffs

@suztomo
Copy link
Member

suztomo commented Sep 23, 2022

We need 1 script generation/generate_diff.sh. The two jobs that run upon updating main branch:

1st GitHub Actions job

steps:
- run: |
  generation/generate_diff.sh
- run: |
  # Commit only Java code.
  git commit ./**/*.java -m 'chore: Java code diff from split repos'
- run: |
  git push -f --set-upstream origin split-repo-diff-non-java

2nd GitHub Actions job

steps:
- run: |
  generation/generate_diff.sh
- run: |
  # Commit non-Java code. How??
  git commit  ???????? -m 'chore: non-java diff from split repos'
- run: |
  git push -f --set-upstream origin split-repo-diff-non-java

suztomo
suztomo previously approved these changes Sep 23, 2022
Comment on lines +80 to +85
git stash
git checkout "${current_branch}"
git stash pop

git checkout -b "${diff_non_java_branch}"
git add .
Copy link
Member

Choose a reason for hiding this comment

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

I like the use of git stash to separete Java change and non-Java changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if you have a better option. I couldn't think besides this that wouldn't require either two jobs or running the script twice.

- name: Run generate-diff script
run: ./generation/generate_diff.sh
env:
USERNAME: ${{ github.actor }}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, #8448 and #8449 both have github-actions force-pushed...

push:
branches:
- main
pull_request:
Copy link
Member

Choose a reason for hiding this comment

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

I assume you're testing. We don't need this job at pull request checks.

Copy link
Contributor Author

@lqiu96 lqiu96 Sep 23, 2022

Choose a reason for hiding this comment

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

Yep! Just added in fa84ab5 so the CI only runs on non-PR push events (should be merge only)

Copy link
Member

Choose a reason for hiding this comment

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

It appears in the checks. Can you fix this?

Screen Shot 2022-09-23 at 4 49 29 PM

Copy link
Contributor Author

@lqiu96 lqiu96 Sep 23, 2022

Choose a reason for hiding this comment

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

Hmm, I'm not sure if I can prevent it from showing up on the checks. It seems to show up on a push event.

I can skip it from running all together.
image

Copy link
Member

Choose a reason for hiding this comment

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

Would you explain why you need this line 19 " pull_request:" ?

Copy link
Member

Choose a reason for hiding this comment

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

I can skip it from running all together.

Yes, please do so. Remove line 19.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good catch! I've removed the check.

@lqiu96 lqiu96 marked this pull request as ready for review September 23, 2022 20:33
@lqiu96 lqiu96 added the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 23, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Sep 23, 2022
@suztomo suztomo dismissed their stale review September 23, 2022 20:56

the job is appearing pull request checks

@lqiu96
Copy link
Contributor Author

lqiu96 commented Sep 26, 2022

I'll merge this now to have it regenerate the diffs on merge. I'll create a separate PR to split the ITs and other Java code.

@lqiu96 lqiu96 merged commit d01ae48 into main Sep 26, 2022
@lqiu96 lqiu96 deleted the main-diff branch September 26, 2022 15:40
suztomo pushed a commit to suztomo/google-cloud-java that referenced this pull request Oct 4, 2022
* chore: Generate diff from current split repos

* chore: Call delete samples script afterwards

* chore: Display diff on GH

* chore: Push diffs to seperate branches

* Revert "chore: Display diff on GH"

This reverts commit 8a616dc.

* chore: Create seperate branches

* chore: Add push to multiple branches

* chore: Stash the non staged changes

* chore: Delete non-generated samples

* chore: Add retry for git clone

* chore: Remove untracked filed

* chore: Run from main-diff branch

* chore: Fix Typo

* chore: cleanup branches

* chore: Test if it can ci works

* chore: Delete branch only if doesn't exist

* chore: Set git username/email

* chore: Use fetch-depth 0 to allow unshallow update

* chore: Only run on merge

* chore: Run action only on merge

* chore: Remove run on pull_request
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.

2 participants