nixpkgs-plugin-update: updated plugin versioning; vimPlugins: update on 2025-12-15#470251
nixpkgs-plugin-update: updated plugin versioning; vimPlugins: update on 2025-12-15#470251MattSturgeon merged 4 commits intoNixOS:masterfrom
Conversation
|
I've not read the python changes enough to properly review them, but I've scanned through a decent chunk of the Consider this an ACK from me if I don't come back to review properly. |
GaetanLepage
left a comment
There was a problem hiding this comment.
Ideally, you could have split the cosmetic changes (log string formatting, scope refacoring...) and the feature addition into two different commits.
Otherwise, the diff looks fine and if it ended up working, we should be good to go!
Yeah looks like I missed ripping some of that out.. I cherry-picked this out from a bigger refactor branch I had just to focus on this specific issue for a smaller PR. |
43ed65b to
eff564e
Compare
|
@GaetanLepage pulled out the unrelated string formatting / redundant else changes. Wanted to keep this as focused on the specific need, as possible. |
MattSturgeon
left a comment
There was a problem hiding this comment.
pulled out the unrelated string formatting / redundant else changes. Wanted to keep this as focused on the specific need, as possible.
Awesome. That makes this much easier to review!
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Outdated
Show resolved
Hide resolved
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Outdated
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
|
PerchunPak
left a comment
There was a problem hiding this comment.
I do appreciate your effort, but the whole system with urllib, threading, mixing 3-5 different ways to get the information about a plugin (including usage of undocumented, probably deprecated, XML API) is doomed to fail. This code is really not sustainable
Any efforts to improve this script will only drive the code more insane. We should really start looking at #380691.
(But that doesn't mean I block this, or any other, PR)
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Outdated
Show resolved
Hide resolved
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Outdated
Show resolved
Hide resolved
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Outdated
Show resolved
Hide resolved
eff564e to
6357b73
Compare
87014a4 to
cc1fc85
Compare
|
Pulled nvim-treesitter stuff out, will be handled in #470883 |
PerchunPak
left a comment
There was a problem hiding this comment.
Aside from cosmetic changes, this is in good shape, and the next review will definitely be an approval from me.
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Outdated
Show resolved
Hide resolved
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Show resolved
Hide resolved
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Show resolved
Hide resolved
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Outdated
Show resolved
Hide resolved
...on-modules/nixpkgs-plugin-update/nixpkgs-plugin-update/src/nixpkgs_plugin_update/__init__.py
Show resolved
Hide resolved
cc1fc85 to
fe06d03
Compare
fe06d03 to
4e25d99
Compare
PerchunPak
left a comment
There was a problem hiding this comment.
Thanks!
Although, first commit message seems wrong, and all the vimPlugins: update should be squashed
4e25d99 to
50c27a0
Compare
You're right, squashed and renamed. |
Match current standards for versioning packages in nixpkgs. Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
Signed-off-by: Austin Horstman <khaneliman12@gmail.com>
50c27a0 to
4a660f8
Compare
GaetanLepage
left a comment
There was a problem hiding this comment.
Thanks @khaneliman!
|
I just realized that version fetching here does not work when there is no token (GraphQL API requires a token). This is the case when people want to add only a single plugin, and it doesn't use the API that much to need a token. I just tested it, and it will set the version to |
Blah that's kinda annoying. Could just fallback to using the git ls-remote approach when no token is provided |
Things done
Updated to fetch tags and rename versions to match latest Nixpkgs versioning naming policy. Will do hash in a follow up to keep diff small and review easier.
Included a package set bump just to show what its generating.
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.