-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Question: what about entries that are focused on performance? #24
Comments
Two things:
The doc specifies an "added", "changed", and "removed" section. What you've proposed is neither adding, nor removing, and it's absolutely a change... so "changed" is where it goes. I see no reason that doesn't adequately contain performance changes. |
I think I agree with @masukomi. Just think of it as Therefore:
The only exception in the guidelines is "Security" because those changes should stand out dramatically. Performance improvements are nice to have, but I think it's a bad idea to dilute attention with one more sub-section when really performance improvements can fit in either: |
Sounds good. |
Overlapping (perhaps duplicate) discussion: #333 . @olivierlacan I see your point that a standalone performance section might be an overkill. Not sure if it changes your mind (I'm undecided too), but I think you're being optimistic in considering only performance improvements. Those are nice to have, but performance regressions are more important to signal in my opinion. |
Hypothetically, what if you had an entries like:
method()
now uses 50% of the memorymethod()
completes in 30% of the previous elapsed timeAre these fixes? Or additions? Should there be a new performance section?
The text was updated successfully, but these errors were encountered: