-
Notifications
You must be signed in to change notification settings - Fork 355
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
[CELEBORN-929][INFRA] Add dependencies check CI #1852
Conversation
572753b
to
832ca91
Compare
Codecov Report
@@ Coverage Diff @@
## main #1852 +/- ##
==========================================
+ Coverage 46.46% 46.52% +0.06%
==========================================
Files 163 164 +1
Lines 10164 10184 +20
Branches 936 936
==========================================
+ Hits 4722 4737 +15
- Misses 5130 5134 +4
- Partials 312 313 +1 see 4 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This reverts commit c692eb2.
510a4ae
to
3815ae2
Compare
GA passed, PTAL. cc @pan3793 @waitinfuture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. cc @waitinfuture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
pull_request: | ||
branches: | ||
- main | ||
- branch-* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the existing release branch does not has the scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since the script dev/dependencies.sh
was introduced in version 0.4.0-SNAPSHOT, which is the current main branch, we don't backport this pull request to an existing release branch, such as branch-0.3. I think it's safe, WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should not backport.
if we merge it as-is to main only, will PR target to branch-0.3 trigger the check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR target to branch-0.3 will not trigger this check, it's the same as SBT CI.
thanks for the review. @SteNicholas @waitinfuture @pan3793 merging to main(v0.4.0). |
What changes were proposed in this pull request?
As title
Why are the changes needed?
As title
Does this PR introduce any user-facing change?
No
How was this patch tested?
Pass GA