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

Give release archive workflows more time #170

Merged
merged 5 commits into from
Oct 11, 2024
Merged

Conversation

ouillie
Copy link
Contributor

@ouillie ouillie commented Oct 8, 2024

Addresses #140.

@ouillie
Copy link
Contributor Author

ouillie commented Oct 8, 2024

Well I tried to craft a test for this, but I've never used jest before. I was surprised to learn that something as basic as a matcher with an arbitrary predicate was something I had to implement myself 🤷 and apparently I didn't do it right. Would highly appreciate somebody with some Typescript / jest experience explaining where I erred.

@ouillie
Copy link
Contributor Author

ouillie commented Oct 8, 2024

Ok, this is now ready for review.

Copy link
Collaborator

@kormide kormide left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the contribution and nice work with the predicate.

Copy link
Collaborator

@kormide kormide left a comment

Choose a reason for hiding this comment

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

@ouillie Are you able to run the e2e tests locally? It looks like the test [snapshot] error email for incorrect strip prefix in multi module repo now times out due to the delay. We'll want a way to set a shorter delay for these tests.

@kormide
Copy link
Collaborator

kormide commented Oct 10, 2024

I would be fine with just setting an env var during testing and checking for it in the release archive code.

@ouillie
Copy link
Contributor Author

ouillie commented Oct 11, 2024

The e2e test fail for me even on the upstream main branch, so I just ignored them on my branch. Is there some setup I'm missing? I'm assuming you guys have some credentials or something that don't get distributed publicly.

@ouillie
Copy link
Contributor Author

ouillie commented Oct 11, 2024

Just cuz I can't run it locally doesn't mean I can't try to fix it though :)

@kormide kormide merged commit 9d4bece into bazel-contrib:main Oct 11, 2024
2 checks passed
@ouillie
Copy link
Contributor Author

ouillie commented Oct 15, 2024

Thanks @kormide. Do you know when this will be available in production? I'm going to wait to kick off my next release until I know it'll work.

@kormide
Copy link
Collaborator

kormide commented Oct 15, 2024

Thanks @kormide. Do you know when this will be available in production? I'm going to wait to kick off my next release until I know it'll work.

I already deployed it so it's ready for your next release!

@ouillie ouillie mentioned this pull request Oct 16, 2024
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