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

Replace lazy_static with once_cell #119

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Yesterday17
Copy link

No description provided.

@hwittenborn
Copy link
Member

Could you merge this repo's master branch into your's so that the CI can run @Yesterday17?

src/control.rs Outdated Show resolved Hide resolved
@LingMan
Copy link
Contributor

LingMan commented Jul 3, 2023

Could you merge this repo's master branch into your's so that the CI can run @Yesterday17?

That would create useless merge commits polluting the history. The correct operation would be to rebase onto current master. Just FYI.

@hwittenborn
Copy link
Member

I was planning on doing a squash merge once the PR got in anyway @LingMan, so it shouldn't matter much.

@LingMan
Copy link
Contributor

LingMan commented Jul 3, 2023

Ah, I see. The squash button does still add the title of the merge commit to the commit message, but it is an improvement over leaving them separate.
Personally I'm always annoyed when maintainers squash my carefully separated commits into one messy pile. Since there's only one real commit here that obviously doesn't apply though. (Not to mention that I'm not the author here, so I'll shut up now.)

@Yesterday17
Copy link
Author

Could you merge this repo's master branch into your's so that the CI can run @Yesterday17?

Done.

@@ -1,5 +1,6 @@
//! A couple of functions to enable and disable coloring.

use once_cell::sync::Lazy;
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming the other crates used in the library have extern crate definitions in the root because of an old Rust version that's being targeted, but I'll have to double check once the crate gets an MSRV figured out (see #85).

This would technically be a breaking change, and I'd probably end up putting this inside a v3 of colored, which is already planned if #139 ends up happening. I'll probably end up making a v2 branch or something, and then I can get this merged in.

@hwittenborn hwittenborn added the breaking Will need a major version bump label Jul 5, 2023
@kurtlawrence
Copy link
Collaborator

Could this been done with https://doc.rust-lang.org/std/sync/struct.Once.html ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Will need a major version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants