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

Get ruby version for graph label from metadata #253

Merged
merged 1 commit into from
May 21, 2024

Conversation

rwstauner
Copy link
Contributor

@rwstauner rwstauner commented May 14, 2024

This way the version is accurate next year, and also if we regenerate old reports.
This implementation uses a placeholder to specify where we want the version (which means labels have to opt in)... We could just append it to all if we want, for example No JIT 3.4.0dev.

before
image

after

image

so that it is always accurate
Comment on lines +56 to +57
["MJIT3.0", @with_mjit30_config],
["MJIT", @with_mjit_latest_config],
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to keep them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not, if we don't care about showing them for past reports.
Currently old reports and data are cached on the server.
If we ever wanted to rebuild past reports for old data files (that had MJIT data), keeping these means they'll be included on the graphs.
If that's not important (and we just wanted old graphs to show whatever configs we still care about now) we could get rid of these (in which case I'd make a new github issue to clean up these references).
What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

We generally bisect commits after the last release (to find a regression point in an under-development version), so it seems fine to have graphs only for dates without MJIT. It should at least cover the past one year.

Having said that, since this only helps maintainability, it seems fine to leave it for now if it's hard to clean them up.

@rwstauner
Copy link
Contributor Author

#256

@rwstauner rwstauner merged commit f77fa63 into main May 21, 2024
1 check passed
@rwstauner rwstauner deleted the rwstauner/version-from-meta branch May 21, 2024 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants