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.

Sign up for GitHub

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

Jump to bottom

Improve test coverage for == operator of std::data #46

Merged
merged 1 commit into from
Mar 18, 2020
Merged

Improve test coverage for == operator of std::data #46

merged 1 commit into from
Mar 18, 2020

Conversation

9prady9
Copy link
Owner

@9prady9 9prady9 commented Mar 18, 2020

Also cleaned up internal code for stl reading functions

@9prady9
Improve test coverage for == operator of std::data
619ca67 
Also cleanedup internal code for stl reading functions
@9prady9 9prady9 requested a review from acrlakshman March 18, 2020 08:32
acrlakshman
acrlakshman reviewed Mar 18, 2020
View reviewed changes
Copy link
Collaborator

@acrlakshman acrlakshman left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@9prady9 I couldn't figure out why we are getting this: https://travis-ci.org/github/9prady9/MeshIO/jobs/663842229#L3984. It even happened locally when building with make only when converage flag is turned on. Any thoughts?

It didn't happen when I used ninja, probably because it suppressed warnings by default.

@9prady9 9prady9 requested a review from acrlakshman March 18, 2020 15:26
@acrlakshman
Copy link
Collaborator

In this line https://travis-ci.org/github/9prady9/MeshIO/jobs/663842229#L3984, I was referring to this /Users/travis/build/9prady9/MeshIO/include/meshio/stl.hpp:79: warning: include file meshio/details/stl.inl not found, perhaps you forgot to add its directory to INCLUDE_PATH?

@acrlakshman
Copy link
Collaborator

I just realized, this warning has been happening for some time and it is present in other builds too, not related to this PR. So merging this now... if you can find a reason, please let me know.

@acrlakshman acrlakshman merged commit d53adf9 into master Mar 18, 2020
@acrlakshman acrlakshman deleted the improv_cov branch March 18, 2020 15:33
@9prady9
Copy link
Owner Author

9prady9 commented Mar 18, 2020

In this line https://travis-ci.org/github/9prady9/MeshIO/jobs/663842229#L3984, I was referring to this /Users/travis/build/9prady9/MeshIO/include/meshio/stl.hpp:79: warning: include file meshio/details/stl.inl not found, perhaps you forgot to add its directory to INCLUDE_PATH?

Yeah, I saw this too. Thought I will handle it over the weekend. Didn't look into what exactly is causing it. Propably some config option in doxygen. But then I again, we don't want to inl documentation to appear in public available docs right ?

@acrlakshman
Copy link
Collaborator

Yes, since .inl includes implementation details (definitions) that are not intended to be part of API, we don't need them included in doxygen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acrlakshman acrlakshman acrlakshman approved these changes

Assignees

@9prady9 9prady9

Labels
cleanup enhancement internal test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@9prady9 @acrlakshman