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.

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

chore!: fix cancel_deferred_save event typo #48

Closed

Conversation

skoch13
Copy link
Contributor

@skoch13 skoch13 commented Mar 19, 2024

No description provided.

Copy link
Collaborator

@primeapple primeapple 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 your contribution. Looks good to me!

Fine for you @okuuva if we merge this?

@primeapple primeapple changed the title chore: fix cancel_deferred_save event typo chore!: fix cancel_deferred_save event typo Mar 22, 2024
@primeapple
Copy link
Collaborator

Ahh, just to keep in mind. This is a breaking change, as it changes the current config, so we need to communicate that.

@okuuva
Copy link
Owner

okuuva commented Mar 25, 2024

I'd love to get this fixed because now that I see the typo I can't unsee it but not sure how to deal with this. It's a breaking change and in theory we should have some grace period where both options work but old option would show a warning during plugin init. But that sounds like quite a hassle just to fix a typo...

Then again, we should have a mechanism for deprecating options, so that we could at some point get rid of execution message dimming and cleaning interval. So I think we should implement that deprecation mechanism first and apply it here before merging.

@skoch13
Copy link
Contributor Author

skoch13 commented Mar 25, 2024

You guys are the bosses here 😄 , still we can show notification when the plugin is loaded about the change, so it should not be that hard to update for those who overridden the defaults.

okuuva added a commit that referenced this pull request May 18, 2024
This removes the execution message and adds a comment to the README on
how to readd them via Autocommands.

Also I implemented a function to handle breaking changes. It can later
be used for #48

This merges in a feature branch. I would like to collect all potential
breaking changes there until we can release it all together in a version
1.0.0

---------

Co-authored-by: okuuva <[email protected]>
@primeapple
Copy link
Collaborator

Hi @skoch13 , to bring this forward, you would have to do the following:

  1. Change the target of this PR to https://github.com/okuuva/auto-save.nvim/tree/first-breaking-changes
  2. Add logic to the handle_deprecations method, so that a warning is given to the user if they use they old cancel_defered_save option. Then, set the value of the new cancel_deferred_save option to the one of the old one and remove the old one.

If you have any questions, I can take this over as well.

@primeapple primeapple changed the base branch from main to first-breaking-changes August 21, 2024 07:45
@skoch13
Copy link
Contributor Author

skoch13 commented Aug 21, 2024

@primeapple thank you for your efforts and my apologies for the delay. I'm not using this plugin at the moment 😬

@primeapple
Copy link
Collaborator

@skoch13 It's fine, I'll merge your commit in #58

@primeapple primeapple closed this Aug 21, 2024
primeapple added a commit that referenced this pull request Oct 9, 2024
Copied over from #48

---------

Co-authored-by: Sergey Kochetkov <[email protected]>
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