Skip to content

Update GTest instructions#4

Open
bennibbelink wants to merge 2 commits into
munkm:cep3-updatesfrom
bennibbelink:gtest
Open

Update GTest instructions#4
bennibbelink wants to merge 2 commits into
munkm:cep3-updatesfrom
bennibbelink:gtest

Conversation

@bennibbelink

Copy link
Copy Markdown

Given our recent updates to the Cyclus build involving GTest, this PR modifies the instructions in CEP3 for updating GTest on release.

@bennibbelink

Copy link
Copy Markdown
Author

@munkm tagging you in this comment since I can't figure out how to request your review for some reason

Comment thread source/cep/cep3.rst Outdated
Comment on lines +342 to +344
**Update GTest:** We fetch the GTest source code during our CMake build. To keep up with
GTest's natural evolution cycle, please reference the latest release of Google Tests
in ``tests/CMakeLists.txt`` If we go too long without doing this,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This looks good! I have a question that would help me clarify for my review:

  • what needs to be updated in tests/CMakeLists.txt? Is it just a single line? Or will several changes need to happen? Maybe it would be clearer to quote the actual argument that needs to be updated in the file?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I just added the command that needs to be updated. Let me know if you think this is sufficient instruction.

@munkm

munkm commented Apr 30, 2024

Copy link
Copy Markdown
Owner

I can't figure out how to request your review for some reason

Thanks for tagging me! It got to the top of my inbox this way.

@bennibbelink bennibbelink requested a review from munkm May 4, 2024 18:39
@bennibbelink

Copy link
Copy Markdown
Author

@munkm do these instructions look sufficient?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants