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

README improvements #648

Merged
merged 3 commits into from
Jul 26, 2018
Merged

README improvements #648

merged 3 commits into from
Jul 26, 2018

Conversation

dmah42
Copy link
Member

@dmah42 dmah42 commented Jul 26, 2018

No description provided.

Copy link
Contributor

@EricWF EricWF left a comment

Choose a reason for hiding this comment

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

Awesome..

README.md Outdated
When the library is built using GCC it is necessary to link with the pthread
library due to how GCC implements `std::thread`. Failing to link to pthread will
lead to runtime exceptions, not linker errors. See [issue #67](https://github.com/google/benchmark/issues/67)
for more details. You can link to pthread by adding `-lpthread` to your linker
Copy link
Contributor

Choose a reason for hiding this comment

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

I think -pthread is preferable. It's like -lpthread but the compiler puts it in the right place. So you can even put the flag before your object files, and they'll still link.

Copy link
Contributor

Choose a reason for hiding this comment

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

will lead to runtime exceptions, not linker errors. See [issue #67]

Hey now! Libc++ has the decency to fail to link!

@coveralls
Copy link

coveralls commented Jul 26, 2018

Coverage Status

Coverage remained the same at 86.962% when pulling 701fe25 on readme into f965eab on master.

Copy link
Collaborator

@LebedevRI LebedevRI left a comment

Choose a reason for hiding this comment

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

Looks good in general, one nit:

README.md Outdated
@@ -123,7 +119,21 @@ Don't forget to inform your linker to add benchmark library e.g. through
`BENCHMARK_MAIN();` at the end of the source file and link against
`-lbenchmark_main` to get the same default behavior.

The benchmark library will reporting the timing for the code within the `for(...)` loop.
The benchmark library will reporting the timing for the code within the
Copy link
Collaborator

Choose a reason for hiding this comment

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

library will reporting the timing

Something is missing between will and reporting.

@dmah42 dmah42 merged commit d939634 into master Jul 26, 2018
@dmah42 dmah42 deleted the readme branch July 26, 2018 13:29
JBakamovic pushed a commit to JBakamovic/benchmark that referenced this pull request Dec 6, 2018
* Clarifications and cleaning of the core documentation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants