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

Add a git-diff line to tools/post_size_changes_to_github.sh #1797

Merged
merged 1 commit into from May 6, 2020

Conversation

gendx
Copy link
Contributor

@gendx gendx commented Apr 24, 2020

Pull Request Overview

I noticed that the size diffing tool has been added back in #1753. However, I find it hard to pinpoint which sections of the code have the most impact in the size difference, because the diff only summarizes global flash and RAM differences.
However, the size reports are more detailed than that.

This pull request adds a simple git diff command over the raw size reports. Although it doesn't compute the difference as a number or percentage, it's a low-hanging fruit which can help understanding where the difference comes from.

Testing Strategy

This pull request was tested by Travis.

TODO or Help Wanted

  • Should this line be part of tools/diff_memory_usage.py instead?

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.

Copy link
Contributor

@hudson-ayers hudson-ayers left a comment

Choose a reason for hiding this comment

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

This looks good; I think that anyone scanning the travis build log for size changes is probably interested in this level of granularity.

@gendx
Copy link
Contributor Author

gendx commented May 5, 2020

Should this be merged or is it superseded by the GitHub workflows?

@hudson-ayers
Copy link
Contributor

I think this should still be merged as long as we are still using Travis, once we move away from Travis this file will be deleted. But it is nice to have the info in both places for now.

@hudson-ayers hudson-ayers added the P-Upkeep This a relatively minor change, or one that is limited in scope, and requires less scrutiny. label May 5, 2020
@hudson-ayers
Copy link
Contributor

bors d+

@bors
Copy link
Contributor

bors bot commented May 5, 2020

✌️ gendx can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@gendx
Copy link
Contributor Author

gendx commented May 6, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented May 6, 2020

@bors bors bot merged commit b3566e9 into tock:master May 6, 2020
@gendx gendx deleted the detailed-size-diff branch May 6, 2020 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-Upkeep This a relatively minor change, or one that is limited in scope, and requires less scrutiny.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants