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

📎 Change bitflags for enumflags2 #3157

Closed
ematipico opened this issue Jun 10, 2024 · 8 comments · Fixed by #3802
Closed

📎 Change bitflags for enumflags2 #3157

ematipico opened this issue Jun 10, 2024 · 8 comments · Fixed by #3802
Labels
good first issue Good for newcomers S-Help-wanted Status: you're familiar with the code base and want to help the project

Comments

@ematipico
Copy link
Member

ematipico commented Jun 10, 2024

Description

Inside the biome_js_analyze, we started using enumflags2, and the DX has been positive.

This is a task to remove bitflags from the codebase, and use enumflags2. This is an example of definition:

#[enumflags2::bitflags]
#[repr(u16)]
pub enum Modifier {
// modifiers must be sorted by precedence.
Decorator = 1 << 0,
BogusAccessibility = 1 << 1,
Private = 1 << 2,
Protected = 1 << 3,
Public = 1 << 4,
Declare = 1 << 5,
Static = 1 << 6,
Abstract = 1 << 7,
Override = 1 << 8,
Readonly = 1 << 9,
Accessor = 1 << 10,
}

Feel free to send multiple PRs to address this, we use bit_flags in different places of the repository.

@ematipico ematipico added good first issue Good for newcomers S-Help-wanted Status: you're familiar with the code base and want to help the project labels Jun 10, 2024
@mvares
Copy link

mvares commented Aug 15, 2024

@ematipico, is there still work to be done here?

@dyc3
Copy link
Contributor

dyc3 commented Aug 15, 2024

Nope, this should have been closed in #3529.

@dyc3 dyc3 closed this as completed Aug 15, 2024
@dyc3
Copy link
Contributor

dyc3 commented Aug 15, 2024

Wait, nevermind, the package name is actually bitflags without the underscore (edited the issue to fix that)

This is where we currently still depend on bitflags: https://github.com/search?q=repo%3Abiomejs%2Fbiome+bitflags+language%3ATOML&type=code&l=TOML

@dyc3 dyc3 reopened this Aug 15, 2024
@dyc3 dyc3 changed the title 📎 Change bit_flags for enumflags2 📎 Change bitflags for enumflags2 Aug 15, 2024
@ematipico
Copy link
Member Author

@ematipico, is there still work to be done here?

@mvares yes, mostly biome_js_parser

There, I suggest porting one bitflag at the time, because there are a lot.

@mvares
Copy link

mvares commented Aug 15, 2024

@ematipico I'll start contribuing to biome. If I'd have some help, can I ask here?

@ematipico
Copy link
Member Author

Of course!

@Javimtib92
Copy link
Contributor

I would like to contribute for the first time to Biome, could I contribute to this issue as well @ematipico ? . I would work on refactoring biome_diagnostics if that's ok to you.

@dyc3
Copy link
Contributor

dyc3 commented Sep 4, 2024

Yes, absolutely. As previously stated though, refactor one flag per PR so it's easier for us to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers S-Help-wanted Status: you're familiar with the code base and want to help the project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants