search

colored-rs / colored

(Rust) Coloring terminal so simple you already know how to do it !
Mozilla Public License 2.0
1.74k stars 84 forks source link

Remove unnecessary lazy_static dependency #176

Open atezet opened 1 month ago

atezet commented 1 month ago

Note that this PR upgrades the MSRV to 1.80

Resolves #175 Closes #119 Closes #170

spenserblack commented 1 month ago

Edited PR body to link to relevant issue

atezet commented 1 month ago

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.

spenserblack commented 1 month ago

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.

atezet commented 1 month ago

It does; but the other one doesn't require upgrading the MSRV, if I'm not mistaken. Still a breaking change nonetheless

kurtlawrence commented 1 month ago

Lgtm!

atezet commented 1 month ago

Thanks! To fix the semver check, should I increase the crate's version? Or is that done in a separate PR

spenserblack commented 1 month ago

Thanks! To fix the semver check, should I increase the crate's version?

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.

kurtlawrence commented 1 month ago

Thanks! To fix the semver check, should I increase the crate's version?

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 :cry:

Thanks @spenserblack for being active in the repo :heart:

atezet commented 1 month ago

Tiny question; shall I make it 2.2.0 or 3.0.0?

spenserblack commented 1 month ago

I've heard some arguments to the contrary, but IMO an increase in the MSRV is a breaking (major) release.

atezet commented 1 month ago

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 commented 1 month ago

@atezet Just a warning that I just force-pushed (formatting nitpick).

atezet commented 1 month ago

Oh, did I reintroduce that with the version bump? Thanks!

spenserblack commented 1 month ago

No problem! I'm guessing it's an auto-formatter at work. My personal trick is to call git add --patch (-p for short). This starts an interactive session that allows you to stage chunks instead of entire files, and also gives you a chance to review the changes. This helps me commit only the changes I want.

atezet commented 1 month ago

Anything needed to get this merged?

spenserblack commented 1 month ago

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.

atezet commented 1 month ago

Any way to reach them?

spenserblack commented 1 month ago

@kurtlawrence Since you've been maintaining before me I don't want to step on any toes. Thoughts on merging this?

spenserblack commented 1 month ago

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 :shrug: If we want to roughly follow this schedule, next year would be when the MSRV would be bumped to 1.80.

atezet commented 4 weeks ago

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).

spenserblack commented 4 weeks ago

I've actually seen the practice of keeping PRs unmerged until shortly before a new release. The linguist repository is an example of that.