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(index): fork index_vec crate. #3092

Merged
merged 7 commits into from
Apr 25, 2024

Conversation

rzvxa
Copy link
Collaborator

@rzvxa rzvxa commented Apr 25, 2024

related to #3086

Copy link
Collaborator Author

rzvxa commented Apr 25, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rzvxa and the rest of your teammates on Graphite Graphite

Copy link

codspeed-hq bot commented Apr 25, 2024

CodSpeed Performance Report

Merging #3092 will not alter performance

Comparing 04-25-chore_index_fork_index_vec_crate (6b7a503) with main (2dd96df)

Summary

✅ 30 untouched benchmarks

@rzvxa rzvxa requested a review from Boshen April 25, 2024 04:12
@rzvxa rzvxa marked this pull request as ready for review April 25, 2024 04:12
@rzvxa
Copy link
Collaborator Author

rzvxa commented Apr 25, 2024

I intentionally didn't add a license file from the original crate so you can review and suggest how are we going to pay attribute to the author.

@Boshen
Copy link
Member

Boshen commented Apr 25, 2024

I intentionally didn't add a license file from the original crate so you can review and suggest how are we going to pay attribute to the author.

I put licenses in https://github.com/oxc-project/oxc/blob/main/THIRD-PARTY-LICENSE

And also mention this is a fork from index_vec on the top of the lib.rs file.

@Boshen
Copy link
Member

Boshen commented Apr 25, 2024

Feel free to merge.

@Boshen Boshen enabled auto-merge (squash) April 25, 2024 06:03
@rzvxa rzvxa force-pushed the 04-25-chore_index_fork_index_vec_crate branch from a01f596 to 6b7a503 Compare April 25, 2024 06:04
@Boshen Boshen merged commit 8618f6b into main Apr 25, 2024
30 checks passed
@Boshen Boshen deleted the 04-25-chore_index_fork_index_vec_crate branch April 25, 2024 06:09
@overlookmotel
Copy link
Collaborator

One bit of feedback: You can avoid compiling the serde dependency when the serialize feature isn't enabled:

In Cargo.toml:

[dependencies]
- serde = { workspace = true }
+ serde = { workspace = true, optional = true }

[features]
- serialize = []
+ serialize = ["dep:serde"]

@rzvxa
Copy link
Collaborator Author

rzvxa commented Apr 25, 2024

@overlookmotel Oh thanks, I forgot to add the serde as optional. I'll fix it.

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.

3 participants