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

treewide: adjust with usage in meta #586

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

RAVENz46
Copy link
Contributor

@RAVENz46 RAVENz46 commented Jan 12, 2025

It follow this suggestion NixOS/nixpkgs#208242 .
We are currently moving away from using with lib; for meta in nixpkgs repo, and this change is in line with that.

Copy link
Owner

@lilyinstarlight lilyinstarlight left a comment

Choose a reason for hiding this comment

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

It's not clear what issue this is fixing at all, so could you include a justification in PR body and/or commit message body?

Otherwise it looks harmless enough to me, thanks!

@RAVENz46
Copy link
Contributor Author

It's not clear what issue this is fixing at all, so could you include a justification in PR body and/or commit message body?

Otherwise it looks harmless enough to me, thanks!

Sorry about that, I added comments.

@lilyinstarlight
Copy link
Owner

Hmm, that nixpkgs issue itself argues "The problem is not when with is used in meta where it is very local and only covering a few lines. I am completely against changing those treewide" and I'm not immediately seeing anything arguing a technical reason to also remove it for meta

But also I don't think it matters here enough to argue about it when this is a trivial change that doesn't seem to harm anything (unlike the nixpkgs treewides which have harmed things, but that's more a process issue over there than anything that matters here)

Thanks for contribution!

@lilyinstarlight lilyinstarlight merged commit faebaec into lilyinstarlight:main Jan 13, 2025
5 checks passed
@lilyinstarlight lilyinstarlight changed the title Stop over-using with lib; treewide: adjust with usage in meta Jan 13, 2025
@RAVENz46
Copy link
Contributor Author

Hmm, that nixpkgs issue itself argues "The problem is not when with is used in meta where it is very local and only covering a few lines. I am completely against changing those treewide" and I'm not immediately seeing anything arguing a technical reason to also remove it for meta

But also I don't think it matters here enough to argue about it when this is a trivial change that doesn't seem to harm anything (unlike the nixpkgs treewides which have harmed things, but that's more a process issue over there than anything that matters here)

Thanks for contribution!

To be honest, I agree with you about this change, but since there are no negative effects from the change, I think it is safe to follow the nixpkgs side.

Anyway, thank you too.

@RAVENz46 RAVENz46 deleted the with-lib branch January 13, 2025 05:28
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