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

[BUG] remove extraneous this.format() from SemVerConstructor #465

Closed
1 task done
kwasimensah opened this issue Jul 22, 2022 · 2 comments
Closed
1 task done

[BUG] remove extraneous this.format() from SemVerConstructor #465

kwasimensah opened this issue Jul 22, 2022 · 2 comments
Labels
Bug thing that needs fixing Needs Triage needs an initial review

Comments

@kwasimensah
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

this.format()
calls this.format()

Expected Behavior

this.format()
should not call this.format()

Steps To Reproduce

this.format()
calls this.format()

Environment

All Environments

@kwasimensah kwasimensah added Bug thing that needs fixing Needs Triage needs an initial review labels Jul 22, 2022
@kwasimensah
Copy link
Author

Actually, format writes to this.version :( I'm hunting down spurious allocations in my project and the call to this.format came up in my debugging.

I'm surprised that this function both reads and writes to a property. It may be better to turn version into a getter that calculates the value on the fly but that might impact people who are relying for it to be calculated?

@lukekarrys
Copy link
Contributor

I don't see this.format() specifically as a performance issue. As you said, it is required to write this.version. I'm going to close this but keep open #458 to track overall reuse of instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs an initial review
Projects
None yet
Development

No branches or pull requests

2 participants