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: remove dist, add release-please #190

Merged
merged 1 commit into from
Oct 12, 2020
Merged

Conversation

bharathkkb
Copy link
Contributor

No description provided.

@google-cla google-cla bot added the cla: yes label Oct 6, 2020
@bharathkkb bharathkkb mentioned this pull request Oct 7, 2020
@bharathkkb bharathkkb changed the title [WIP] feat: remove dist, add release-please feat: remove dist, add release-please Oct 9, 2020
@averikitsch
Copy link
Contributor

If we remove dist/ before we run release-please this will break the actions

.github/workflows/appengine-deploy-it.yml Show resolved Hide resolved
.github/workflows/appengine-deploy-it.yml Outdated Show resolved Hide resolved
.github/workflows/deploy-cloudrun-it.yml Outdated Show resolved Hide resolved
.github/workflows/setup-gcloud-it.yml Show resolved Hide resolved
@averikitsch averikitsch self-assigned this Oct 9, 2020
@bharathkkb
Copy link
Contributor Author

If we remove dist/ before we run release-please this will break the actions

@averikitsch so my understanding is that when we merge this to main because of https://github.com/GoogleCloudPlatform/github-actions/blob/e829c1b7b9c2f632d89189668ec8348f5270bfc3/.github/workflows/release-please.yml#L5 this should be triggered and will proceed to recreate those dists and commit to main, so yes technically we will be broken on main for a couple of minutes (or longer if we run into any errors during dist build on main). I will do a final test after I address the comments above with a fresh fork.

@bharathkkb
Copy link
Contributor Author

Oh I think I get what you meant, dont remove existing dist, let it autogenerate and commit instead. That would indeed work better, the reason I removed it here is because I was testing that the JIT build for int tests do work. I will re add them back.

@averikitsch
Copy link
Contributor

Oh I think I get what you meant, dont remove existing dist, let it autogenerate and commit instead. That would indeed work better, the reason I removed it here is because I was testing that the JIT build for int tests do work. I will re add them back.

Yes this sounds good. Note to self, leave better comments.

@bharathkkb
Copy link
Contributor Author

@averikitsch did another build release test here with current workflow https://github.com/bharathkkb/github-actions/actions/runs/298346191

@bharathkkb bharathkkb merged commit 11f1439 into master Oct 12, 2020
@bharathkkb bharathkkb deleted the new-build-process branch October 12, 2020 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants