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

Avoid copying examples when building tests #281

Merged
merged 7 commits into from
Feb 11, 2025
Merged

Conversation

jwallwork23
Copy link
Collaborator

Closes #279.

Simplifies the developer experience, as discussed in today's meeting.

  • FTorch/src/CMakeLists.txt moved to FTorch/CMakeLists.txt.
  • FTorch/src/tests/ moved to FTorch/tests/.
  • No longer any need to copy the examples when building FTorch with test support.
  • Updated docs accordingly.
  • Added API change note.

@jwallwork23 jwallwork23 added the testing Related to FTorch testing label Feb 10, 2025
@jwallwork23 jwallwork23 self-assigned this Feb 10, 2025
@jwallwork23 jwallwork23 changed the title 279 avoid copy paste Avoid copying examples when building tests Feb 10, 2025
@jwallwork23 jwallwork23 marked this pull request as ready for review February 10, 2025 17:48
Copy link
Member

@jatkinson1000 jatkinson1000 left a comment

Choose a reason for hiding this comment

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

Hi @jwallwork23 CMake adjustments look fine, but I think you need to update the installation instructions on the main README and in the pages/ directory to reflect that users no longer build in src/build/.

Would be worth a comb through the rest of the READMEs and docs to check for anything else that may need changing.

@jwallwork23
Copy link
Collaborator Author

Hi @jwallwork23 CMake adjustments look fine, but I think you need to update the installation instructions on the main README and in the pages/ directory to reflect that users no longer build in src/build/.

Would be worth a comb through the rest of the READMEs and docs to check for anything else that may need changing.

Thanks @jatkinson1000 - I think I covered everything with e0e85d3. Grepped for src and updated anything needed.

Copy link
Member

@jatkinson1000 jatkinson1000 left a comment

Choose a reason for hiding this comment

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

Thanks @jwallwork23 On the assumption you catched 'em all I'm happy to approve this.

@jwallwork23 jwallwork23 merged commit c85185e into main Feb 11, 2025
6 checks passed
@jwallwork23 jwallwork23 deleted the 279_avoid-copy-paste branch February 11, 2025 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Related to FTorch testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid copying examples when building tests
2 participants