Skip to content

vim-configurable: rename from vim_configurable#145351

Closed
Artturin wants to merge 1 commit intoNixOS:masterfrom
Artturin:vimconfigurablerename
Closed

vim-configurable: rename from vim_configurable#145351
Artturin wants to merge 1 commit intoNixOS:masterfrom
Artturin:vimconfigurablerename

Conversation

@Artturin
Copy link
Copy Markdown
Member

Motivation for this change
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@Artturin Artturin mentioned this pull request Nov 10, 2021
@github-actions github-actions bot added 6.topic: vim Advanced text editor 8.has: documentation This PR adds or changes documentation labels Nov 10, 2021
@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package labels Nov 10, 2021
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Nov 10, 2021
@teto
Copy link
Copy Markdown
Member

teto commented Nov 10, 2021

could you reference the document that mandate it please ?

@Artturin
Copy link
Copy Markdown
Member Author

Dashes are now valid as part of identifiers and attribute names.

https://nixos.org/manual/nix/stable/release-notes/rl-1.2.html?highlight=dash#release-12-2012-12-06

most of the packages that use _ in the name are probably pre-2012

this isn't a upstream package so the below doesn't apply to this

Dashes in the package name should be preserved in new variable names, rather than converted to underscores or camel cased — e.g., http-parser instead of http_parser or httpParser. The hyphenated style is preferred in all three package names.

https://nixos.org/manual/nixpkgs/stable/#sec-package-naming

@Artturin
Copy link
Copy Markdown
Member Author

Artturin commented Nov 12, 2021

any objections to merging? @teto

@teto
Copy link
Copy Markdown
Member

teto commented Nov 13, 2021

Looks like useless churn to me, especially as it's a nixpkgs specific function. If we have to change it, maybe we can can change it to vimWrapper to align it with neovim wrapper ? maybe it's not a wrapper though if it changes vim compilation options though.

@millerjason
Copy link
Copy Markdown
Contributor

I expect a number of people use vim_configurable today for customization. Even with the alias, these types of changes are best accomplished as a holistic upgrade across all legacy packages to the new convention. That way users can follow a single rule rather than having to consider variants that might change piecemeal over releases.

The quoted text does not seem to imply that there is imminent need for this remediation, although the new-style convention does appear more prominently. Is this part of a broader effort to do a cleanup or is there some other motivation?

@Artturin
Copy link
Copy Markdown
Member Author

is there some other motivation?

#145344 (comment)

Then why vim_configurable rather vim.configurable or vim-configurable?

@Artturin Artturin force-pushed the vimconfigurablerename branch from 7b46d90 to c998dcc Compare November 19, 2021 23:11
@Artturin Artturin force-pushed the vimconfigurablerename branch from c998dcc to 7358b93 Compare November 19, 2021 23:14
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes labels Nov 19, 2021
@ofborg ofborg bot requested a review from millerjason November 19, 2021 23:34
@ncfavier
Copy link
Copy Markdown
Member

Is there any reason to have two separate derivations for vim and vim_configurable? From what I've seen, vim could be defined from the configurable version by disabling most things. Then I think it would make more sense to rename vim_configurable to something like vimFull or vim-full, to align with the other packages (and make vimHugeX and vim_configurable aliases). Tell me if this makes sense.

@smancill
Copy link
Copy Markdown
Contributor

Is this still considered or abandoned?

@ncfavier
Copy link
Copy Markdown
Member

ncfavier commented Dec 4, 2022

Superseded by #204438

@Artturin Artturin closed this Dec 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: vim Advanced text editor 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants