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

feat!: Remove deprecated execution message #54

Merged
merged 5 commits into from
May 18, 2024

Conversation

primeapple
Copy link
Collaborator

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

@primeapple primeapple self-assigned this May 6, 2024
@primeapple primeapple force-pushed the feat!-remove-execution-message branch from 12b4e8b to 3ab64b0 Compare May 6, 2024 19:50
@primeapple
Copy link
Collaborator Author

Hi @okuuva ,
I'd like to get your opinion on this one. Especially if we should completely remove the execution_message or if we should just remove the coloring.

I did the first here, because an execution message is a perfect example for an Autocommand, that the user can define themselves. But I'm open to other opinions.

Next thing, is there any other breaking change to add (besides #48 )? I thought about letting the user specify the save command (would remove the config options write_all_buffers, noautocmd and lockmarks). But we can also keep them to keep it simple for the user.

Copy link
Owner

@okuuva okuuva left a comment

Choose a reason for hiding this comment

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

Great job, as always!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
lua/auto-save/config.lua Show resolved Hide resolved
lua/auto-save/utils/colors.lua Outdated Show resolved Hide resolved
@okuuva
Copy link
Owner

okuuva commented May 13, 2024

Hi @okuuva , I'd like to get your opinion on this one. Especially if we should completely remove the execution_message or if we should just remove the coloring.

I did the first here, because an execution message is a perfect example for an Autocommand, that the user can define themselves. But I'm open to other opinions.

I think we should just axe it altogether. The whole functionality is reinventing the wheel and I have a feeling the most common first edit to default config is to disable the message. I personally like it but that's what AutoCommands are for.

Next thing, is there any other breaking change to add (besides #48 )? I thought about letting the user specify the save command (would remove the config options write_all_buffers, noautocmd and lockmarks). But we can also keep them to keep it simple for the user.

Let's start with this and #48 and make the save command configurable in the next major version. I don't have any real justifications for this, it's just that I'd like to start small when introducing breaking changes. If for nothing else then to give people time to bind their plugin version to v1 before breaking more things. After a sufficient time after v1 we can introduce breaking changes as often as we like. Version numbers are cheap after all.

Copy link
Owner

@okuuva okuuva left a comment

Choose a reason for hiding this comment

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

It's not ideal that we can't use the same syntax for all package managers when it comes to version binding but I still think we should by default instruct people to pin their plugin version to the latest version using the syntax the different tools support.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@primeapple
Copy link
Collaborator Author

I applied your suggestions @okuuva . If anything doesn't work with plug or packer, we can always update it. Also we might want to remove packer alltogether instead of pckr.nvim. I honestly would rather add https://github.com/nvim-neorocks/rocks.nvim imho that's more the direction of the ecosystem.

But we can do this in a separate task. Let's merge this :)

@okuuva okuuva merged commit 1747cf2 into first-breaking-changes May 18, 2024
2 checks passed
@okuuva okuuva deleted the feat!-remove-execution-message branch May 18, 2024 05:46
balvig added a commit to balvig/.dotfiles that referenced this pull request Oct 19, 2024
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.

2 participants