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

windows 0.51 #32

Merged
merged 2 commits into from
Oct 12, 2023
Merged

windows 0.51 #32

merged 2 commits into from
Oct 12, 2023

Conversation

kayabaNerve
Copy link
Contributor

While this compiles, I'm unsure if it effects msrv/if Some(callback) is as expected. I'd hope the CI's tests to catch any errors, as I do not have a windows machine to actually run this one.

@mxinden
Copy link
Owner

mxinden commented Sep 26, 2023

Thank you for the patch.

While this compiles, I'm unsure if it effects msrv/if Some(callback) is as expected.

In what way would this pull request be a breaking change? In other words, what of windows is exposed through if-watch?

While this compiles, I'm unsure if it effects msrv/if Some(callback) is as expected. I'd hope the CI's tests to catch any errors, as I do not have a windows machine to actually run this one.

Me neither. Do you know anyone? What was your initial motivation to propose this patch?

@kayabaNerve
Copy link
Contributor Author

kayabaNerve commented Sep 26, 2023

It'd be a breaking change if a party expects if-watch to run on Rust 1.xy, which windows 0.34 does, yet windows 0.51 does not. I didn't evaluate the msrv of windows to see if windows 0.51 increased its msrv when compared to 0.34, and am unsure what if-watch's current/advertised msrv is.

The motivation for the patch was to reduce tree size. Right now, I have several windows crates in tree which no other dependency uses as they're over a year old. I then also have windows 0.48 in tree by project(s) which do maintain their windows dependencies (please note there was no 0.49 nor 0.50, and 0.51 is only a month old). It's just unappreciated to have both contribute to my disk space and compile times. While I could've been greedy and simply updated if-watch to 0.48, for my immediate happiness, I'd rather everyone in the ecosystem do their best to stay up to date.

This was solely an attempt to be that person keeping things up to date, rather than silently expecting random people on the internet do so.

I have no explicit reason to believe this necessary for function.

Regarding the Some introduced, I believe that's proper so long as the callback from if-watch is non-null, which I believe it to be. Regarding testing, I believe GH CI offers Windows runners, though it sounds like that's not configured by your reply?

@kayabaNerve
Copy link
Contributor Author

@mxinden The CI does test against a Windows host, so this should be verified if you let the CI run.

Copy link
Owner

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thank you for the help!

@mxinden mxinden merged commit 9b87056 into mxinden:master Oct 12, 2023
7 checks passed
@mxinden
Copy link
Owner

mxinden commented Oct 12, 2023

Tagged and published.

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