Skip to content
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

plugins/persistence: convert to mkNeovimPlugin and fix settings #2805

Merged
merged 1 commit into from
Jan 8, 2025
Merged

plugins/persistence: convert to mkNeovimPlugin and fix settings #2805

merged 1 commit into from
Jan 8, 2025
+53 −79

Conversation

jolars
Copy link
Contributor

@jolars jolars commented Jan 8, 2025

  • Converted the plugin to use mkNeovimPlugin
  • Removed some arguments that have been removed upstream and have no effect. I wasn't sure how to handle this. Right now enabling those options throws an error, but maybe there should just be a warning?

GaetanLepage
GaetanLepage requested changes Jan 8, 2025
View reviewed changes
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.

Thanks for this !

mkRemovedOptionModule is the right choice for this indeed.

plugins/by-name/persistence/default.nix Outdated Show resolved Hide resolved
plugins/by-name/persistence/default.nix Show resolved Hide resolved
@jolars jolars force-pushed the convert-persistence branch from 938afe3 to f9408ba Compare January 8, 2025 13:58
@jolars jolars force-pushed the convert-persistence branch from f9408ba to b401927 Compare January 8, 2025 14:25
@jolars @GaetanLepage
plugins/persistence: convert to mkNeovimPlugin and fix settings
8899663 
Restrict no nonnegative numbers

Co-authored-by: Gaétan Lepage <33058747+GaetanLepage@users.noreply.github.com>

chore: include optionsRenamedToSettings in TODO

Merge imports

Co-authored-by: Gaétan Lepage <33058747+GaetanLepage@users.noreply.github.com>
@jolars jolars force-pushed the convert-persistence branch from 335cff3 to 8899663 Compare January 8, 2025 14:56
GaetanLepage
GaetanLepage approved these changes Jan 8, 2025
View reviewed changes
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.

LGTM, thanks :)

@GaetanLepage
Copy link
Member

@mergify queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 8, 2025 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 8899663

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 8, 2025

This pull request, with head sha 8899663b5928c1326a4d8f721430da7e947f4dc1, has been successfully merged with fast-forward by Mergify.

This pull request will be automatically closed by GitHub.

As soon as GitHub detects that the sha 8899663b5928c1326a4d8f721430da7e947f4dc1 is part of the main branch, it will mark this pull request as merged.

It is possible for this pull request to remain open if this detection does not happen, this usually happens when a force-push is done on this branch convert-persistence, this means GitHub will fail to detect the merge.

@mergify mergify bot merged commit 8899663 into nix-community:main Jan 8, 2025
4 checks passed
@jolars jolars deleted the convert-persistence branch January 9, 2025 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaetanLepage GaetanLepage GaetanLepage approved these changes

@MattSturgeon MattSturgeon MattSturgeon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jolars @GaetanLepage @MattSturgeon