vimPlugins: nvimRequireChecks and nvimSkipModules for new nvimRequireCheckHook#360800
Merged
khaneliman merged 4 commits intoNixOS:masterfrom Dec 1, 2024
Merged
vimPlugins: nvimRequireChecks and nvimSkipModules for new nvimRequireCheckHook#360800khaneliman merged 4 commits intoNixOS:masterfrom
khaneliman merged 4 commits intoNixOS:masterfrom
Conversation
Some plugins have modules that require setup called or a config/opts. Just requiring the main module instead of creating a large list of skips.
nix-owners
bot
requested review from
GaetanLepage,
PerchunPak,
figsoda,
manveru,
mrcjkb,
rvolosatovs and
teto
github-actions
bot
added
10.rebuild-darwin: 11-100
GaetanLepage
approved these changes
Dec 1, 2024
Contributor
|
We dont want to bundle nvim-cmp with these because of alternative completion plugins existing that can utilize these sources. Adding nvim-cmp to run checkphase, though.
We dont want to bundle nvim-web-devicons with these because of alternative plugins existing that can mock their require. Adding nvim-web-devicons to run checkphase, though.
khaneliman
force-pushed
the
neovim-require-check
branch
from
e6fc76a to
6af0ef2
Compare
Contributor
Author
|
Just updated note on |
Member
|
It's unusual to start merging unused code. As I feared the number of false positives is concerning. almost 700 lines (and that is without #352277 (comment)) that doesn't bring much in terms of robustness. |
Contributor
Author
Not sure I understand what the comment means. This was just pulling out the code changes that were already tested in #352277 so we dont cause constant conflicts in the github actions for master -> staging-next -> staging cycle due to the The linked comment are build failures. What is that in relation to this work? |
This was referenced Dec 1, 2024
Member
|
If I understand correctly, this PR does nothing without #352277 ?
Are you talking about pkgs/applications/editors/vim/plugins/overrides.nix ? in my experience it's pretty straightforward to rebase, the chunks are easily identifiable and it's part of the PR process.
Sry for the link, I just meant the PR in general, not the specific comment my bad |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Things done
Pulling out the vimPlugins changes from #352277 so that plugin additions/changes don't cause merge conflicts constantly during staging cycle. They wont have any effect until the hook changes make it to master.
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.