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

Spilt Tagged<T> into a utility crate #3849

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Spilt Tagged<T> into a utility crate #3849

wants to merge 1 commit into from

Conversation

HalidOdat
Copy link
Member

@HalidOdat HalidOdat commented May 12, 2024

Depends on #3837

This makes Tagged<T> be usable in boa_string and boa_engine.

@HalidOdat HalidOdat added the Internal Category for changelog label May 12, 2024
@HalidOdat HalidOdat added this to the v0.18.1 milestone May 12, 2024
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 50,731 50,731 0
Passed 42,973 42,973 0
Ignored 1,395 1,395 0
Failed 6,363 6,363 0
Panics 18 18 0
Conformance 84.71% 84.71% 0.00%

@nekevss
Copy link
Member

nekevss commented May 13, 2024

General discussion on utility crates: how do we want to go about handling utilities in general that's going to be best for maintenance in the long term?

Should we have one general boa_utils crate or a utils directory with individual crates hosted inside it (I'm thinking of icu4x's structure as an example). We could also extract SmallMap into the utils as well. Both Tagged and SmallMap could then be standalone crates unto themselves.

There's some benefits to taking the utils directory approach.

  1. The code is more modular.
  2. Offers the libraries for generic use case for the Rust ecosystem as a whole.
  3. Could provide some soft "marketing" for boa, so to speak.

There could be a pretty big negative around the maintenance complexity utility crates may cause and managing separate releases may actually end being a large headache. 😕

Any thoughts?

@HalidOdat
Copy link
Member Author

Hmmm... Tagged<T> and SmallMap are very small (single file) so don't think it's going to need that much maintenance. Just a thought: we could put them in separate repos (like we do with ryu-js, and temporal_rs) may make it easier to separately publish the crates.

What does the rest of the team say? @jedel1043 @raskad @Razican

@Razican
Copy link
Member

Razican commented May 13, 2024

From my side, in general, if the crate / code makes sense outside Boa, then we should make it separate, so that others can benefit from it. If not, I would keep it in.

For this case, it seems these types could be used outside of Boa, and they have no dependency on the engine, so I would make them separate if it's not too much maintenance burden (and we don't forget to maintain them 😅)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Category for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants