Remove unnecessary lazy_static dependency #176
Conversation
|
Edited PR body to link to relevant issue |
Cargo.toml
Outdated
| "Win32_Foundation", | ||
| "Win32_System_Console", | ||
| ] | ||
| features = ["Win32_Foundation", "Win32_System_Console"] |
There was a problem hiding this comment.
I'm personally pretty nitpicky about unnecessary noise in diffs.
For example, if I used git blame to find who created these features, the blame would pick you if this PR was merged.
There was a problem hiding this comment.
Removed this change in a force push
atezet
force-pushed
the
remove-lazy-static
branch
from
5ed2bda
to
d37f68f
Compare
|
Thanks! About when to merge; I'm not sure either. I'll leave that to you if that's okay. I didn't notice that there was already an issue for this. |
|
No problem! I'll leave this PR open to give other maintainers a chance to offer their thoughts. This PR conflicts with #170, but I'm thinking that this PR looks a little better. |
|
It does; but the other one doesn't require upgrading the MSRV, if I'm not mistaken. Still a breaking change nonetheless |
|
Lgtm! |
|
Thanks! To fix the semver check, should I increase the crate's version? Or is that done in a separate PR |
Good question! @kurtlawrence What I've been doing in a personal project is increasing the crate version whenever a PR makes a change that would bump it. It makes it pretty easy to release since your crate version is already at the correct incrementation, and you just have to compare the current crate version to the latest release if you need to double-check if you need to bump the version. |
I don't particularly mind. This crate doesn't see a lot of changes, so increasing/publishing with each PR is probably fine and would avoid having main go too far out of date with crates.io I think if I was more active with this repo I'd try to bundle a bunch of PRs into a single release, but unfortunately I don't have much time to spend on this 😢 Thanks @spenserblack for being active in the repo ❤️ |
|
Tiny question; shall I make it |
|
I've heard some arguments to the contrary, but IMO an increase in the MSRV is a breaking (major) release. |
|
Bumped the version to 3.0.0. However, I noticed clippy mentioned a lot of things to me; would you want me to fix these (in a separate PR) as well before merging this one and bumping the version to 3.0.0 (some of them break backwards compatibility)? I'd happily do it as I'm working on the code already anyway |
spenserblack
force-pushed
the
remove-lazy-static
branch
from
0bb8291
to
250b791
Compare
|
@atezet Just a warning that I just force-pushed (formatting nitpick). |
|
Oh, did I reintroduce that with the version bump? Thanks! |
|
No problem! I'm guessing it's an auto-formatter at work. My personal trick is to call |
|
Anything needed to get this merged? |
It looks good, but I'm personally not merging yet because I'm not sure how other maintainers feel about this making it in to the next release. |
|
Any way to reach them? |
|
@kurtlawrence Since you've been maintaining before me I don't want to step on any toes. Thoughts on merging this? |
|
On December 9th we bumped up minimum version 1.70 (released June 2023). At that time 1.74 was already out. So you could say that we support roughly 6 months back, or 4 minor releases back 🤷 If we want to roughly follow this schedule, next year would be when the MSRV would be bumped to 1.80. |
|
That makes sense. Although I feel like it wouldn't really matter if there aren't any other updates in the meantime (which you'll never know of course). |
|
I've actually seen the practice of keeping PRs unmerged until shortly before a new release. The linguist repository is an example of that. |
Note that this PR upgrades the MSRV to 1.80
Resolves #175
Closes #119
Closes #170