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: remove ci main workflow #12232

Merged
merged 2 commits into from
Sep 12, 2023
Merged

chore: remove ci main workflow #12232

merged 2 commits into from
Sep 12, 2023

Conversation

huangjw806
Copy link
Contributor

@huangjw806 huangjw806 commented Sep 12, 2023

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Because we now have a pull-request pipeline and a merge-queue, we think it is not necessary to run the main pipeline again after the merge. In addition, we also run the main-cron pipeline every day.

After removing the main pipeline, we can save at least 20% of CI costs.

  • Migrate unique jobs in main pipeline to main-cron pipeline;
  • Migrate release and docker jobs in main pipeline to docker pipeline;
  • Remove main pipeline;
  • enable docker pipeline Github tag webhook active;

Releated issue: close #12230

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
  • All checks passed in ./risedev check (or alias, ./risedev c)
  • My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)
  • My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

@kwannoel
Copy link
Contributor

I guess this has added bonus of only having to make sure 2 pipelines are stable rather than monitoring 3 different ones.

The main thing to note is that we lose test coverage on release build for each main commit. Now only 1 main commit at the end of the day will be tested via main-cron.

I think we can merge it, see how it performs. If there's many errors which are only found in main / main-cron, then we should revert, to catch these errors per main commit, OR in merge queue instead of running pull-request workflow, we run main workflow instead.

@huangjw806
Copy link
Contributor Author

I guess this has added bonus of only having to make sure 2 pipelines are stable rather than monitoring 3 different ones.

In fact, we are not paying attention to every failed main pipeline now. In the future, we only need to strictly pay attention to the failed main-cron pipeline.

@kwannoel
Copy link
Contributor

I guess this has added bonus of only having to make sure 2 pipelines are stable rather than monitoring 3 different ones.

In fact, we are not paying attention to every failed main pipeline now. In the future, we only need to strictly pay attention to the failed main-cron pipeline.

You are right, it is not strictly paid attention to. I think this would be the alternative then, since it would block PRs from merging unless it passes:

OR in merge queue instead of running pull-request workflow, we run main workflow instead.

@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #12232 (52ced32) into main (43852dc) will decrease coverage by 0.01%.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #12232      +/-   ##
==========================================
- Coverage   69.73%   69.73%   -0.01%     
==========================================
  Files        1409     1409              
  Lines      235961   235961              
==========================================
- Hits       164553   164552       -1     
- Misses      71408    71409       +1     
Flag Coverage Δ
rust 69.73% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@huangjw806 huangjw806 force-pushed the jianwei/remove-main-workflow branch from d6e8069 to 52ced32 Compare September 12, 2023 09:40
@huangjw806 huangjw806 added this pull request to the merge queue Sep 12, 2023
Merged via the queue into main with commit 7078959 Sep 12, 2023
6 of 7 checks passed
@huangjw806 huangjw806 deleted the jianwei/remove-main-workflow branch September 12, 2023 10:36
Li0k pushed a commit that referenced this pull request Sep 15, 2023
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.

remove per PR test or use the test on main for merge queue.
3 participants