Skip to content

Conversation

FKouhai
Copy link
Member

@FKouhai FKouhai commented Oct 7, 2025

initial work done:

  • Moved coverage over to mkNeovim
  • Rewritten parts of the test, moving the older options over inside the settings
  • tested locally by running nix develop --interactive and picking coverage

@FKouhai FKouhai marked this pull request as draft October 7, 2025 06:37
@FKouhai
Copy link
Member Author

FKouhai commented Oct 7, 2025

introduced by: #2638

@FKouhai FKouhai force-pushed the coverage_mknvim branch 3 times, most recently from e1052f9 to edf4477 Compare October 10, 2025 21:38
@FKouhai
Copy link
Member Author

FKouhai commented Oct 10, 2025

I think this is ready for review, though I probably need some help to understand the deprecations.nix file, I was taking a look at some others but couldn't really make a lot of sense out of it, so definitely would appreciate the help

image

@FKouhai FKouhai marked this pull request as ready for review October 10, 2025 21:40
@FKouhai FKouhai changed the title [WIP]plugins/coverage: migrate to mkNeovim plugins/coverage: migrate to mkNeovim Oct 10, 2025
Copy link
Member

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

Thank you for giving this a try.

A few remarks:

  • You need to properly deprecate the old keymaps and keymapsSilent options as they do not map to a new settings.X option. (Use mkRemovedOptionModule).
    They should not be in optionsRenamedToSettings. This is made for options that were mapped to the plugin's native options (i.e. autoReload -> auto_reload).
  • You need to import the deprecations.nix (see
    # TODO: introduced 2025-07-16: remove after 25.11
    inherit (import ./deprecations.nix lib) deprecateExtraOptions optionsRenamedToSettings;
    )

@FKouhai FKouhai force-pushed the coverage_mknvim branch 2 times, most recently from 8d7c6af to b2be75d Compare October 11, 2025 09:36
@FKouhai
Copy link
Member Author

FKouhai commented Oct 11, 2025

Thank you for giving this a try.

A few remarks:

  • You need to properly deprecate the old keymaps and keymapsSilent options as they do not map to a new settings.X option. (Use mkRemovedOptionModule).
    They should not be in optionsRenamedToSettings. This is made for options that were mapped to the plugin's native options (i.e. autoReload -> auto_reload).
  • You need to import the deprecations.nix (see
    # TODO: introduced 2025-07-16: remove after 25.11
    inherit (import ./deprecations.nix lib) deprecateExtraOptions optionsRenamedToSettings;

    )

Thank you! yeah after taking a deeper look into the upstream repo I see that I didnt really need keymaps nor keymapsSilent :)

@FKouhai FKouhai force-pushed the coverage_mknvim branch 2 times, most recently from a5b62f3 to 5fbec45 Compare October 11, 2025 17:56
@FKouhai
Copy link
Member Author

FKouhai commented Oct 11, 2025

just rebased this branch FYI

Copy link
Member

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

I pushed a minor stylistic change.
Thanks a lot @FKouhai for your patience!

I think that this is PR is now good to be merged :)

@FKouhai
Copy link
Member Author

FKouhai commented Oct 11, 2025

I pushed a minor stylistic change. Thanks a lot @FKouhai for your patience!

I think that this is PR is now good to be merged :)

awesome! thank you very much for your reviews

@khaneliman khaneliman added this pull request to the merge queue Oct 12, 2025
Merged via the queue into nix-community:main with commit 37451a5 Oct 12, 2025
4 checks passed
@FKouhai FKouhai deleted the coverage_mknvim branch October 12, 2025 07:43
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.

3 participants