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

[upstream_utils] Add googletest #6820

Merged
merged 10 commits into from
Jul 19, 2024
Merged

[upstream_utils] Add googletest #6820

merged 10 commits into from
Jul 19, 2024

Conversation

pjreiniger
Copy link
Contributor

@pjreiniger pjreiniger commented Jul 9, 2024 •
edited
Loading

Fixes #6811

Adds gtest to the source tree.

  • Gradle Build
  • Cmake Build
  • Artifact Publishing

calcmogul
calcmogul previously requested changes Jul 9, 2024
View reviewed changes
Copy link
Member

@calcmogul calcmogul left a comment

Choose a reason for hiding this comment

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

We use thirdparty instead of third_party for all the other upstream_utils imports.

.styleguide Outdated Show resolved Hide resolved
@pjreiniger
Copy link
Contributor Author

So in its current state this splits include / src as the rest of the projects do. However, that means that we can't pull in the CMakeList.txt from the repo and I had to make my own, which spider webbed out quite a bit.

Would it be OK to leave the file structure how it is natively? If so I can undo the split and remove a lot of the extra churn

@pjreiniger pjreiniger marked this pull request as ready for review July 11, 2024 06:02
@pjreiniger pjreiniger requested review from PeterJohnson and a team as code owners July 11, 2024 06:02
PeterJohnson
PeterJohnson previously approved these changes Jul 17, 2024
View reviewed changes
@PeterJohnson PeterJohnson changed the title [upstream utils] Add gtest [upstream_utils] Add googletest Jul 17, 2024
PeterJohnson
PeterJohnson reviewed Jul 17, 2024
View reviewed changes
thirdparty/googletest/publish.gradle

artifactId = baseArtifactId
groupId artifactGroupId
version wpilibVersioning.version.get()
Copy link
Member

Choose a reason for hiding this comment

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

@ThadHouse this will change versioning from the current thirdparty-googletest, correct? Is that an issue at all, or does this actually make things easier?

Copy link
Member

Choose a reason for hiding this comment

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

Makes things slightly easier, although it will require a new version of native-utils to actually pick up. Otherwise the old one will be picked up, which will be fine for now because its a static only library, and not part of the ABI of anything shipped.

calcmogul
calcmogul previously requested changes Jul 17, 2024
View reviewed changes
Copy link
Member

@calcmogul calcmogul left a comment

Choose a reason for hiding this comment

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

This needs to be rebased to use the new upstream_utils API.

@PeterJohnson PeterJohnson merged commit f561a77 into wpilibsuite:main Jul 19, 2024
29 of 30 checks passed
@pjreiniger pjreiniger deleted the upstream_gtest branch July 19, 2024 04:46
@pjreiniger pjreiniger mentioned this pull request Jul 20, 2024
Closed
@spacey-sooty spacey-sooty mentioned this pull request Jan 1, 2025
Merged
spacey-sooty added a commit to PhotonVision/photonvision that referenced this pull request Jan 1, 2025
@spacey-sooty
Fix googletest dependency (#1666)
2a82393 
This was rendered useless when googletest was added to the allwpilib
monorepo wpilibsuite/allwpilib#6820

Signed-off-by: Jade Turner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KangarooKoala KangarooKoala KangarooKoala left review comments

@ThadHouse ThadHouse ThadHouse left review comments

@calcmogul calcmogul calcmogul left review comments

@PeterJohnson PeterJohnson PeterJohnson left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move GoogleTest into the monorepo via upstream_utils
5 participants
@pjreiniger @PeterJohnson @calcmogul @ThadHouse @KangarooKoala