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

Deprecate expandShorthands/createShorthands/create*Shorthand/expand*Shorthand #512

Closed
oliverklee opened this issue Mar 1, 2024 · 11 comments
Assignees

Comments

@oliverklee
Copy link
Contributor

oliverklee commented Mar 1, 2024

#510 (comment)

Expanding and creating the shorthand notation is out of scope for this library. So we should remove the functionality, make all the corresponding methods a no-op and mark them these methods as @deprecated #511 will be removed in version 10.

@JakeQZ
Copy link
Contributor

JakeQZ commented Mar 2, 2024

We could provide a link in the readme to a tag for the last release which includes the functionality, to help anyone who wants to include it in their own project, and/or encourage someone to build it into a separate package.

@sabberworm
Copy link
Contributor

@ziegenberg IMHO you could have put the deprecations into a single PR instead of doing a new PR for every single method. Thanks a lot for your contributions, though.

@oliverklee
Copy link
Contributor Author

IMHO you could have put the deprecations into a single PR instead of doing a new PR for every single method.

@ziegenberg talked/wrote about this, and I requested them to be single PRs. This allows for smaller PRs and allows us to easily revert single things that haven't worked out.

@ziegenberg
Copy link
Contributor

I surely would have put them into one PR. Would have been much easier and would require a lot less rebasing, namely 0.

That's another thing to put into the CONTRIBUTING.md, together with all other kinds of policies.

@sabberworm
Copy link
Contributor

It seems @oliverklee and I disagree about what is an atomic change…

@sabberworm
Copy link
Contributor

I guess a compromise would have been to make it a single PR but one commit for each change. That would have made them revertable without all the drawbacks.

@oliverklee
Copy link
Contributor Author

Then we would have had a merge commit, and I find a linear history really helpful.

For cases like this, I'd be fine with having separate PRs for the deprecations and a follow-up PR to add the changelog entries. This would help avoid the merge conflicts and the need for a rebase. Would that be helpful for you, @ziegenberg?

@ziegenberg
Copy link
Contributor

Then we would have had a merge commit, and I find a linear history really helpful.

For cases like this, I'd be fine with having separate PRs for the deprecations and a follow-up PR to add the changelog entries. This would help avoid the merge conflicts and the need for a rebase. Would that be helpful for you, @ziegenberg?

Yes, that would be helpful next time.

This time, Whether I rebase this pile of PRs or collect all changelog entries from every PR, package it into a new PR and adjust all PRs, it makes no difference in the work I have to do.

In either case, make it a policy and put it into CONTRIBUTING.md for others to follow.

@ziegenberg
Copy link
Contributor

Can be closed now.

@oliverklee
Copy link
Contributor Author

Thanks a lot, @ziegenberg! ❤️

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

No branches or pull requests

4 participants