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

Address unexpected_cfgs warnings instead of allowing the lint #243

Merged
merged 1 commit into from
May 24, 2024

Conversation

Urgau
Copy link
Contributor

@Urgau Urgau commented May 24, 2024

@Lokathor
Copy link
Owner

so does this deny the spirv check?

or does it allow the check which is expected to be deny by default?

@Urgau
Copy link
Contributor Author

Urgau commented May 24, 2024

It denies the lint (aka #![deny(unexpected_cfgs)]) and expect/allow the target_arch = "spirv" cfg (so it can be used without triggering the lint).

@Lokathor
Copy link
Owner

...okay.

Not very good key names, but it's already on the way to stable so i guess I'll just ignore yet another part of rust

@Lokathor Lokathor merged commit f9f7bb1 into Lokathor:main May 24, 2024
14 checks passed
@Urgau
Copy link
Contributor Author

Urgau commented May 24, 2024

Not very good key names, but it's already on the way to stable so i guess I'll just ignore yet another part of rust

check-cfg matches rustc --check-cfg flag

@Lokathor
Copy link
Owner

I guess I'm just not familiar with that area. To the uninitiated it doesn't read well.

@Urgau
Copy link
Contributor Author

Urgau commented May 24, 2024

If your interested in knowing more, here is the Cargo specific doc and here the rustc --check-cfg doc.

The output of the lint links directly to the Cargo specific doc.

To the uninitiated it doesn't read well.

If you have ideas on how to improve the situation I'm happy to here them.

@Lokathor
Copy link
Owner

Well like I said by now it's already stable in rustc the next release, and in cargo the release after that. I don't think anyone would seriously entertain any changes at this point.

zachs18 pushed a commit to zachs18/bytemuck that referenced this pull request Jun 6, 2024
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