Skip to content

vimPlugins: fix meta shadowing #438707

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 30, 2025
Merged

vimPlugins: fix meta shadowing #438707

merged 1 commit into from
Aug 30, 2025
+88 −64

Conversation

khaneliman
Copy link
Contributor

Currently shadows entire meta from generated.nix.

Discovered in nix-community/nixvim#3648

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 6.topic: vim Advanced text editor labels Aug 30, 2025
@MattSturgeon

This comment was marked as resolved.

Sign in to view
@khaneliman
Copy link
Contributor Author

khaneliman commented Aug 30, 2025 •
edited
Loading

Some of those are using buildVimPlugin, I have another Pr i'm creating for moving those to non-generated. I'll try and clean up the others that are redefining meta in overrides I'll draft #438708 real quick and switch back to finishing the meta task.

@khaneliman
Copy link
Contributor Author

Alright, think I got em all.

@MattSturgeon

This comment was marked as resolved.

Sign in to view
@khaneliman
Copy link
Contributor Author

Oops, yeah. I was just searching for meta = last round of changes.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 2 This PR was reviewed and approved by two persons. label Aug 30, 2025
@khaneliman
vimPlugins: fix meta shadowing
6b1069c 
Currently shadows entire `meta` from `generated.nix`.

Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
@khaneliman
Copy link
Contributor Author

third times the charm? Did a grug-far in that file for meta usage and it looks like they are all being merged with old.meta now (other than the buildVimPlugin usages.

MattSturgeon
MattSturgeon approved these changes Aug 30, 2025
View reviewed changes
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

SGTM, thanks!

Q: can we trust eval when it comes to vimPlugins or does the hydraPlatforms = [] mess with it?
It would be a shame to accidentally merge an eval failure if one of those old.metas doesn't actually exist...

EDIT: I guess seeing as hydraPlatforms is part of meta, CI must be able to evaluate meta in order to know there are no hydra platforms... Logically, we must be ok.

@khaneliman
Copy link
Contributor Author

Yeah, I was worried about that too, at first. But our new plugin additions from generated.nix will always have a meta of some kind, specifically because of that hydraPlatforms meta.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. and removed 12.approvals: 2 This PR was reviewed and approved by two persons. labels Aug 30, 2025
@MattSturgeon MattSturgeon merged commit cf80d94 into NixOS:master Aug 30, 2025
29 of 32 checks passed
@MattSturgeon
Copy link
Contributor

Thanks! 🎉

@khaneliman khaneliman deleted the vim-meta branch August 30, 2025 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MattSturgeon MattSturgeon MattSturgeon approved these changes

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@GaetanLepage GaetanLepage GaetanLepage approved these changes

@mrcjkb mrcjkb Awaiting requested review from mrcjkb

@PerchunPak PerchunPak Awaiting requested review from PerchunPak

Assignees
No one assigned
Labels
6.topic: vim Advanced text editor 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 3+ This PR was reviewed and approved by three or more persons.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@khaneliman @MattSturgeon @SuperSandro2000 @GaetanLepage