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

Util fromArcToBeziers() API update #6733

Merged
merged 3 commits into from
Dec 19, 2020
Merged

Util fromArcToBeziers() API update #6733

merged 3 commits into from
Dec 19, 2020

Conversation

jasonsturges
Copy link
Contributor

@jasonsturges jasonsturges commented Dec 16, 2020

Proposing API change:

  • Currently: fromArcToBeizers
  • Proposing: fromArcToBeziers

@asturur
Copy link
Member

asturur commented Dec 18, 2020

i have to check how old is that util, in case we can make a correct alias, and deprecate the old one, but not remove it right away.

@jasonsturges
Copy link
Contributor Author

@asturur Good idea, sorry - hadn't thought of that.

Looks like it was added August 20, 2020
SHA: 1c2164b
Pull Request: #6504

Would you like me to add a deprecation?

Sorry for the hassle - certainly no major issue here, if you'd like to close and consider this some other time.

@asturur
Copy link
Member

asturur commented Dec 19, 2020

you can correct the function name and export, then add an alias with the wrong old name and a @deprecated notic in the jsdoc, with a line explaining that was a typo

@jasonsturges
Copy link
Contributor Author

jasonsturges commented Dec 19, 2020

@asturur Faster than me - just beat me to it.

I had added:

image

Which I think is working as intended:

image

But I won't push, as it looks like we did the same thing.

@jasonsturges
Copy link
Contributor Author

Removed some whitespace and had to revert the change I made (sorry, I had pushed at the same time).

I think this is good now if you want to review the final pull request.

@asturur
Copy link
Member

asturur commented Dec 19, 2020

I added the suggestion to save you typing!
Thanks for the PR merging now

@asturur asturur merged commit a03f52e into fabricjs:master Dec 19, 2020
shanicerae pushed a commit to shanicerae/fabric.js that referenced this pull request Jan 16, 2021
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