Skip to content

Conversation

Manishearth
Copy link
Member

@Manishearth Manishearth commented Jun 9, 2021

This refactors Makefile.toml into multiple subfiles. It also adds various ci-job-foo make tasks which are directly run by CI, so adding further CI can be done by tweaking Makefile.toml only.

This reduces the number of parallel CI jobs. We still have separate jobs for things that are fast-fail: there's a separate check, tidy, fmt, and clippy job, but otherwise there's a "test" (for most tests), "features" (for the "all feature combos" test), and "wasm" jobs. Jobs like "bincode" have been rolled into "test": unless the job has a reason to be long running or require weird dependencies, it should be a part of "test".

Fixes #612

@Manishearth Manishearth requested a review from a team as a code owner June 9, 2021 23:33
@Manishearth Manishearth requested review from echeran and sffc June 9, 2021 23:33
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .github/workflows/build-test.yml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .cargo/config.toml is different
  • .github/workflows/build-test.yml is different
  • tools/scripts/wasm.toml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@Manishearth Manishearth closed this Jun 9, 2021
@Manishearth Manishearth reopened this Jun 9, 2021
@Manishearth
Copy link
Member Author

image

looks better now!

@codecov-commenter
Copy link

codecov-commenter commented Jun 10, 2021

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.96%. Comparing base (2a47704) to head (21eaa16).
Report is 3768 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #783      +/-   ##
==========================================
- Coverage   76.17%   75.96%   -0.22%     
==========================================
  Files         189      193       +4     
  Lines       12151    12234      +83     
==========================================
+ Hits         9256     9293      +37     
- Misses       2895     2941      +46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coveralls
Copy link

coveralls commented Jun 10, 2021

Pull Request Test Coverage Report for Build fe550cc9d36fb76896f423396775865f3de03dd1-PR-783

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 76.119%

Totals Coverage Status
Change from base Build da5bfbfdfa8dc7946530ec3ec97b365e1ab627c7: 0.0%
Covered Lines: 9384
Relevant Lines: 12328

💛 - Coveralls

sffc
sffc previously approved these changes Jun 10, 2021
@Manishearth
Copy link
Member Author

I made it cache cargo-make, but I plan to write a github action that makes that easy to do. The existing install GHA supports caching, but badly (it uses an external cache), and using the builtin cache is blocked on actions-rs/install#5

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • .cargo/config.toml is different
  • Makefile.toml is different
  • tools/scripts/tidy.toml is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • components/icu/src/lib.rs is no longer changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@Manishearth Manishearth requested a review from sffc June 10, 2021 02:40
@Manishearth
Copy link
Member Author

This is ready to land, though before we land this we need to update the required CI runs in the repo settings

@Manishearth
Copy link
Member Author

I tried cleaning up the cargo make installs in #787, but it didn't work. Eventually actions-rs/install will get support for the native cache.

sffc
sffc previously approved these changes Jun 10, 2021
# This is a cargo-make file included in the toplevel Makefile.toml

[tasks.tidy]
[tasks.tidy-minus-fmt]
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion (optional): Delete the "tidy-minus-fmt" task, move these up into "ci-job-tidy", and make "tidy" invoke fmt + ci-job-tidy.

Copy link
Member Author

Choose a reason for hiding this comment

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

I kinda want the ci-job-foo to be toplevel so you can change CI jobs without changing other stuff.

Copy link
Member Author

Choose a reason for hiding this comment

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

eh, fixed it anyway

@sffc
Copy link
Member

sffc commented Jun 10, 2021

When we're ready to merge this, we should change the branch permissions to reflect the new CI.

@Manishearth
Copy link
Member Author

Seems like I have to change the rules before the push, so we have to wait for these checks to pass and I need another r+

@Manishearth Manishearth merged commit 17385b2 into unicode-org:main Jun 10, 2021
@Manishearth Manishearth deleted the makefile-refactor branch June 10, 2021 14:23
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.

Refactor Makefile.toml into smaller files

5 participants