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

Report profiler version #251

Merged
merged 12 commits into from
Sep 11, 2020
Merged

Report profiler version #251

merged 12 commits into from
Sep 11, 2020

Conversation

lptr
Copy link
Member

@lptr lptr commented Sep 10, 2020

Expose the profiler version in the HTML report and on the command-line.

Fixes #171

@lptr lptr added this to the 0.15.0 milestone Sep 10, 2020
@lptr lptr self-assigned this Sep 10, 2020
@lptr lptr changed the title Report additional info Report profiler version Sep 11, 2020
@lptr lptr marked this pull request as ready for review September 11, 2020 14:03
@lptr lptr requested a review from wolfs September 11, 2020 14:03
Copy link
Member

@wolfs wolfs left a comment

Choose a reason for hiding this comment

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

Looks mostly good, I added some comments.

Could you upload a benchmark.html file, so I can look at it without running an actual benchmark?

build.gradle.kts Outdated
manifest {
attributes(
"Implementation-Title" to "Gradle Profiler",
"Implementation-Version" to archiveVersion
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to use project.version here, since we may remove the version from the archive name.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, fixed.

"definition": {
stringWriter.toString() ==~ /\{
"environment": \{
"profilerVersion": ".*"
Copy link
Member

Choose a reason for hiding this comment

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

The version should be available here, so you could assert the actual version string, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is UNKNOWN if you try to run the test from classes. But... yeah, we could do it that way.

@lptr lptr requested a review from wolfs September 11, 2020 14:21
Copy link
Member

@wolfs wolfs left a comment

Choose a reason for hiding this comment

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

LGTM!

@lptr lptr merged commit 00e126b into master Sep 11, 2020
@lptr lptr deleted the lptr/report-additional-info branch September 11, 2020 20:01
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.

Record version number
2 participants