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

Reestructure repo and CI improvements #3505

Merged
merged 8 commits into from
Dec 6, 2023
Merged

Reestructure repo and CI improvements #3505

merged 8 commits into from
Dec 6, 2023

Conversation

jedel1043
Copy link
Member

Our number of crates is slowly growing, and it's getting harder to find the exact crate that you want to open when working on the repo. This PR proposes:

  • Removing the boa_ prefix on all directories, since that's mostly superfluous.
  • Separating our crates in groups:
    • core for the essential crates.
    • tests for test crates.
    • ffi for FFI related crates (only wasm for now).
    • examples, which is just a plain crate in the root for accessibility.
    • cli which is just a plain crate for simplicity.

This also adds some small CI improvements, like better VSCode launch and task scripts, and some cache actions that weren't caching cargo installs.

@jedel1043 jedel1043 added technical debt rust Pull requests that update Rust code Internal Category for changelog labels Dec 6, 2023
@jedel1043 jedel1043 added this to the v0.18.0 milestone Dec 6, 2023
@jedel1043 jedel1043 requested a review from a team December 6, 2023 04:50
@nekevss
Copy link
Member

nekevss commented Dec 6, 2023

I actually like this change a lot! One early question: is there a reason for core over crates. I think bevy, wasmtime, rust-analyzer, and cargo all use a crates directory, and I had assumed, perhaps wrongly, that that was the semi "standard" after a certain point on a project.

@jedel1043
Copy link
Member Author

I actually like this change a lot! One early question: is there a reason for core over crates. I think bevy, wasmtime, rust-analyzer, and cargo all use a crates directory, and I had assumed, perhaps wrongly, that that was the semi "standard" after a certain point on a project.

It's just personal preference, and crates is not really standard per se. ICU4X uses a similar structure as the proposed one, with a components folder with the public APIs and some secondary directories such as utils, ffi, tools, etc.

Copy link

github-actions bot commented Dec 6, 2023

Test262 conformance changes

Test result main count PR count difference
Total 95,609 95,960 +351
Passed 76,526 76,534 +8
Ignored 18,132 18,477 +345
Failed 951 949 -2
Panics 0 0 0
Conformance 80.04% 79.76% -0.28%
Fixed tests (4):
test/intl402/Locale/likely-subtags.js [strict mode] (previously Failed)
test/intl402/Locale/likely-subtags.js (previously Failed)
test/intl402/Locale/prototype/minimize/removing-likely-subtags-first-adds-likely-subtags.js [strict mode] (previously Failed)
test/intl402/Locale/prototype/minimize/removing-likely-subtags-first-adds-likely-subtags.js (previously Failed)
Broken tests (2):
test/built-ins/RegExp/unicode_full_case_folding.js [strict mode] (previously Passed)
test/built-ins/RegExp/unicode_full_case_folding.js (previously Passed)

Copy link

codecov bot commented Dec 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (main@6f5c25c). Click here to learn what that means.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3505   +/-   ##
=======================================
  Coverage        ?   49.01%           
=======================================
  Files           ?      469           
  Lines           ?    48186           
  Branches        ?        0           
=======================================
  Hits            ?    23618           
  Misses          ?    24568           
  Partials        ?        0           

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

Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

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

Nice change! :)

@HalidOdat HalidOdat requested a review from a team December 6, 2023 22:38
Copy link
Member

@nekevss nekevss left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@@ -10,7 +10,7 @@
//!
//! [proposal]: https://github.com/tc39/proposal-temporal
//! [spec]: https://tc39.es/proposal-temporal/
#![doc = include_str!("../../ABOUT.md")]
#![doc = include_str!("../ABOUT.md")]
Copy link
Member

Choose a reason for hiding this comment

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

Nice catch on this!

@jedel1043 jedel1043 added this pull request to the merge queue Dec 6, 2023
Merged via the queue into main with commit 47351ef Dec 6, 2023
14 checks passed
@HalidOdat HalidOdat deleted the cleanup-repo-dirs-ci branch December 6, 2023 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Category for changelog rust Pull requests that update Rust code technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants