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

Bring back NO_COLOR support. #5034

Merged
merged 2 commits into from
Dec 14, 2023
Merged

Bring back NO_COLOR support. #5034

merged 2 commits into from
Dec 14, 2023

Conversation

djl
Copy link
Member

@djl djl commented Dec 7, 2023

Fixes #5028.

@JOJ0
Copy link
Member

JOJ0 commented Dec 10, 2023

Thanks for the fix @djl, please add a quick changelog entry.

@djl
Copy link
Member Author

djl commented Dec 10, 2023

I didn't add a changelog entry for this as the original entry from when it was added is still there and there hasn't been a release with the removal yet. Does it make sense to add a changelog entry for this?

@JOJ0
Copy link
Member

JOJ0 commented Dec 10, 2023

Fun fact: I had similar thoughts: when do we need changelog, when not. I thought we had this fearure in 1.6.0 already though. didnt check.

help @Serene-Arc @RollingStar in that case we can skip the changelog right?

@Serene-Arc
Copy link
Contributor

If it hasn't been released then yes, I think the changelog can be skipped. Its purpose is to tell users/developers what has changed.

@JOJ0
Copy link
Member

JOJ0 commented Dec 11, 2023

@Serene-Arc agreed that actually we wouldn't need the entry, but I think I've made up my mind on what I think would be bestin this situation: Dear @djl please add just the bug/issue ID to the existing changelog entry for the UI overhaul. That way we make sure we link to all the relevant pieces of that particular entry in the changelog (at the very top "major ..." list):

https://beets.readthedocs.io/en/latest/changelog.html#in-development

@RollingStar we recently talked about stuff like that elsewhere. The changelog is currently our best bet to record PR talk alongside actual code changes. If we don't add this "extra PR/fix" to the changelog, this information would get lost well... not super easy to find - git blame / git lense would probably find it. But it simply wouldn't show up if you were to investigate quickly using the changelog on an investigation trying to answer "what happened exactly when the UI was redisgned"

Certainly with that approach there would be some "after UI overhaul fixes" be missing at exactly that changelog entry. I could fix that if we decide it's the way to go.

Does that idea make sense to you all?

@djl
Copy link
Member Author

djl commented Dec 12, 2023

Done!

@JOJ0
Copy link
Member

JOJ0 commented Dec 14, 2023

I think this is safe to merge. Fellow maintainers can still comment on my general "how to changelog question"

@JOJ0 JOJ0 merged commit bf6eb04 into master Dec 14, 2023
14 checks passed
@snejus snejus deleted the YES_NO_COLOR branch June 15, 2024 03:43
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.

Bring back NO_COLOR support?
3 participants