search

rust-windowing / glutin

A low-level library for OpenGL context creation
Apache License 2.0
1.98k stars 476 forks source link

Update `raw-window-handle` to v0.6 #1670

Closed declantsien closed 3 months ago

declantsien commented 6 months ago
kchibisov commented 6 months ago

We already have #1582 for that, though we have an option for 0.6 just for the time being unless I have time to look into the safe handle stuff and how it could be approached in glutin.

kchibisov commented 6 months ago

I'd just post a review, since there's nothing wrong with just using raw version of 0.6, since it's just small update.

MarijnS95 commented 6 months ago

A lot of changes here may snoop the implementation from https://github.com/ash-rs/ash/pull/799 which - to my knowledge - treats (non)-nullable parameters accordingly.

sarthakvk commented 5 months ago

Is there a plan to merge this PR? @declantsien

kchibisov commented 5 months ago

In general, re-request the review with github UI when it's ready or explicitly indicate the state if you're not interested anymore.

declantsien commented 4 months ago

@kchibisov Can you help with the CI error. Wired cfg errors.

declantsien commented 4 months ago

@kchibisov Can you help with the CI error. Wired cfg errors.

The error is from the rust nightly. Tried with an old build nightly-2024-04-05 the errors are gone. Since we don't have a rust-toolchain.toml file, no idea how to deal with it. Another question, why don't we use Cargo.lock? Cargo.lock is useful for reproducible build.

kchibisov commented 4 months ago

Another question, why don't we use Cargo.lock? Cargo.lock is useful for reproducible build.

The old default behavior for rust was to not include Cargo.lock for libraries. They changed that to include last year iirc.

declantsien commented 4 months ago

Should be good after rebase.

Nice

declantsien commented 4 months ago

Another question, why don't we use Cargo.lock? Cargo.lock is useful for reproducible build.

The old default behavior for rust was to not include Cargo.lock for libraries. They changed that to include last year iirc.

Right. Good to know. Thanks.

MarijnS95 commented 4 months ago

Another question, why don't we use Cargo.lock? Cargo.lock is useful for reproducible build.

A reproducible build (for libraries) is "useless" when a published crate is on crates.io. Only semver version (range) dependencies from Cargo.toml are taken into account when your crate is used in another project. cargo doesn't magically "merge" its Cargo.lock with the lockfile of the current project:

https://doc.rust-lang.org/cargo/faq.html#:~:text=However%2C%20this%20determinism%20can%20give%20a%20false%20sense%20of%20security%20because%20Cargo.lock%20does%20not%20affect%20the%20consumers%20of%20your%20package%2C%20only%20Cargo.toml%20does%20that.

MarijnS95 commented 4 months ago

One item remaining, the rest looks good:

declantsien commented 4 months ago

One item remaining, the rest looks good:

@MarijnS95 I think I am done with your requests.