Skip to content

More bitflags #243

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

Closed
wants to merge 5 commits into from
Closed

More bitflags #243

wants to merge 5 commits into from

Conversation

fiveop
Copy link
Contributor

@fiveop fiveop commented Jan 23, 2016

This patch converts some constants to bitflags types. It should cover all flags in src/sched.rs and src/sys/mman.rs. This is part of resolving #58.

use libc::{self, c_int};

bitflags!{
flags MmapFlags: c_int {
Copy link
Member

Choose a reason for hiding this comment

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

could we call these MapFlags for consistency?

@kamalmarhubi
Copy link
Member

Hi @fiveop! This is great, thanks for taking the time. I just have a couple of naming suggestions, because the naming of most flags seems to be XYZ_ -> XyzFlags.

@fiveop
Copy link
Contributor Author

fiveop commented Jan 27, 2016

I agree and will push a corresponding change. In addition, I will create a file which states this convention (in which we may than explicitly state further conventions) and open a PR for discussion.

@kamalmarhubi
Copy link
Member

@fiveop +++ on the discussion + convention doc! This PR made me think o fit but haven't followed through.

@kamalmarhubi
Copy link
Member

I'll wait for your extra commit then we can merge it.

@fiveop
Copy link
Contributor Author

fiveop commented Jan 28, 2016

I had to rebase, so I'll open a new PR for tests.

@fiveop fiveop closed this Jan 28, 2016
@fiveop fiveop mentioned this pull request Jan 28, 2016
@kamalmarhubi
Copy link
Member

For future reference, rebase and force push will trigger tests just fine. :-)

@fiveop fiveop deleted the more_bitflags branch January 29, 2016 08:17
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.

2 participants