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

feat: adds script for git migration. #10882

Merged
merged 12 commits into from
Apr 20, 2023
Merged

Conversation

atulep
Copy link
Contributor

@atulep atulep commented Apr 3, 2023

Script that migrates Git history of a split repo to google-cloud-python.

It copies Git history from source repo (i.e. "split repo") to target repo (i.e. google-cloud-python). Then, creates a pull request in target repo.

@atulep atulep marked this pull request as ready for review April 7, 2023 01:54
@atulep atulep requested a review from a team as a code owner April 7, 2023 01:54
scripts/split_repo_migration/git-migrate-history.sh Outdated Show resolved Hide resolved
scripts/split_repo_migration/git-migrate-history.sh Outdated Show resolved Hide resolved
scripts/split_repo_migration/git-migrate-history.sh Outdated Show resolved Hide resolved
scripts/split_repo_migration/run.sh Outdated Show resolved Hide resolved
scripts/split_repo_migration/run.sh Outdated Show resolved Hide resolved
scripts/split_repo_migration/run.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

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

One relatively minor comment.

But I do think we want the other open comment, deriving ${ARTIFACT_NAME} from the split repo.

scripts/split_repo_migration/git-migrate-history.sh Outdated Show resolved Hide resolved
@atulep atulep requested a review from vchudnov-g April 18, 2023 17:53
@atulep atulep requested a review from vchudnov-g April 19, 2023 00:53
Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

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

Looks good. Only issue is about the commented-out gh command. Could you address that before merging?

scripts/split_repo_migration/git-migrate-history.sh Outdated Show resolved Hide resolved
scripts/split_repo_migration/git-migrate-history.sh Outdated Show resolved Hide resolved
$EXIT 1
fi

pushd "${TARGET_REPO}" # To target repo
Copy link
Contributor

Choose a reason for hiding this comment

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

As per offline chat, sounds like we also need to mkdir -p owl-bot-staging/${DISTRIBUTION_NAME} after validation and before creating the PR.

Which means maybe we do the jq extract into ${DISTRIBUTION_NAME} and then set TARGET_PATH="packages/${DISTRIBUTION_NAME}" above?

Copy link
Contributor

Choose a reason for hiding this comment

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

As per correction in chat, we'll need to

mkdir -p owl-bot-staging/${DISTRIBUTION_NAME}/${DISTRIBUTION_NAME}
touch owl-bot-staging/${DISTRIBUTION_NAME}/${DISTRIBUTION_NAME}/${DISTRIBUTION_NAME}.txt
git add owl-bot-staging
git commit -m "Trigger owlbot post-processor"

although it's not clear this is sufficient to make the migrated repo pass pre-submits.

Copy link
Contributor Author

@atulep atulep Apr 19, 2023

Choose a reason for hiding this comment

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

Ok, added owl-bot-staging. But, we should add any necessary fixes, if needed, in follow-up PRs.

scripts/split_repo_migration/git-migrate-history.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@vchudnov-g vchudnov-g left a comment

Choose a reason for hiding this comment

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

One non-trivial comment about making the title have a conventional commit format.

scripts/split_repo_migration/git-migrate-history.sh Outdated Show resolved Hide resolved
@atulep atulep merged commit 0cd2938 into main Apr 20, 2023
@atulep atulep deleted the monorepo_migration_git_migration_script branch April 20, 2023 01:38
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