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

Validate added migration file #1772

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tkdchen
Copy link
Contributor

@tkdchen tkdchen commented Dec 20, 2024

STONEBLD-2827

Before you complete this pull request ...

Look for any open pull requests in the repository with the title "e2e-tests update" and
see if there are recent e2e-tests updates that will be applicable to your change.

@tkdchen tkdchen force-pushed the STONEBLD-2827-setup-ci-for-migration branch 3 times, most recently from 23818af to f0fe270 Compare December 23, 2024 07:16
@tkdchen tkdchen force-pushed the STONEBLD-2827-setup-ci-for-migration branch from f0fe270 to bf71ad7 Compare December 23, 2024 07:18
@tkdchen tkdchen force-pushed the STONEBLD-2827-setup-ci-for-migration branch 12 times, most recently from f8216f0 to 35693f1 Compare December 24, 2024 03:24
@tkdchen tkdchen marked this pull request as ready for review December 24, 2024 03:34
@tkdchen tkdchen requested a review from a team as a code owner December 24, 2024 03:34
hack/validate-migration.sh Show resolved Hide resolved
hack/validate-migration.sh Outdated Show resolved Hide resolved
hack/validate-migration.sh Outdated Show resolved Hide resolved
@tkdchen tkdchen force-pushed the STONEBLD-2827-setup-ci-for-migration branch from 3419ce7 to e5b84a2 Compare January 6, 2025 09:08
@tkdchen tkdchen requested a review from chmeliik January 7, 2025 04:07
case "$status" in
A) # file is added
task_dir_path=$(awk -F '/' '{ OFS = "/"; print $1, $2 }' <<<"$origin_path")
if grep -q "$task_dir_path" <<<"$seen"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will cause problems

$ grep -q "task/buildah" <<< "task/buildah-remote task/some-other-task" && echo already seen                                                       
already seen

Maybe change the seen variable to be multiline and check like this?

grep -q "^${task_dir_path}$" <<< "$seen"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Updated.

hack/validate-migration.sh Show resolved Hide resolved
@tkdchen tkdchen force-pushed the STONEBLD-2827-setup-ci-for-migration branch 2 times, most recently from ebd6ba2 to 3821d6e Compare January 9, 2025 06:03
@tkdchen
Copy link
Contributor Author

tkdchen commented Jan 9, 2025

Rebased this PR in order to base on the fix #1788

@tkdchen tkdchen requested a review from chmeliik January 9, 2025 06:43
Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

LGTM, could you just squash the commits?

STONEBLD-2827

A few checks are introduced by this commit. Please refer to individual
check_ functions for detailed information.

Signed-off-by: Chenxiong Qi <[email protected]>
@tkdchen tkdchen force-pushed the STONEBLD-2827-setup-ci-for-migration branch from 3821d6e to a55f38d Compare January 9, 2025 08:56
@tkdchen
Copy link
Contributor Author

tkdchen commented Jan 9, 2025

Squash is done.

@tkdchen
Copy link
Contributor Author

tkdchen commented Jan 9, 2025

/test build-definitions-pull-request

@tkdchen
Copy link
Contributor Author

tkdchen commented Jan 10, 2025

/test build-definitions-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.

3 participants