Skip to content

Conversation

@antond-weta
Copy link
Contributor

Switch to using external repo for camera data.

@antond-weta antond-weta requested a review from kdt3rd July 1, 2025 02:20
@antond-weta antond-weta marked this pull request as draft July 1, 2025 23:29
@antond-weta
Copy link
Contributor Author

converting this to a draft for now, as I'd like to make some more changes to the data repo

@antond-weta antond-weta marked this pull request as ready for review July 14, 2025 07:05
@KelSolaar
Copy link
Contributor

Sorry, coming late (I was on a break). Is there a specific reason for not using a Git sub-module here that is pulled when cloning the repo?

@antond-weta
Copy link
Contributor Author

Sorry, coming late (I was on a break). Is there a specific reason for not using a Git sub-module here that is pulled when cloning the repo?

I don't have a specific reason, as this is the first time I'm doing this. I believe Kimball had advised me against using sub-modules here (unless I misunderstood badly).

@KelSolaar
Copy link
Contributor

KelSolaar commented Jul 17, 2025

It would be awesome for @kdt3rd to comment as this is adding quite a bit of code for functionality that we get for free basically with Git.

Signed-off-by: Anton Dukhovnikov <[email protected]>
Signed-off-by: Anton Dukhovnikov <[email protected]>
Signed-off-by: Anton Dukhovnikov <[email protected]>
@antond-weta
Copy link
Contributor Author

@KelSolaar I would suggest merging this as is for now, if you don't have any other concerns. I understand that we can switch to using git sub-modules later if needed, the change is self-contained and does not affect anything else.

A couple of arguments in support of the current solution off the top of my head:

  • fetching the data-sources in cmake is more flexible, we can switch the repos conditionally; we may want, for example, pull additional camera models from the physlight repo if the matching cmake flag is set.
  • fetching the data-sources in cmake we can check the compatible schema version and pick the corresponding tag

@KelSolaar
Copy link
Contributor

Certainly fine by me!

@antond-weta antond-weta merged commit cd0eab2 into AcademySoftwareFoundation:master Jul 21, 2025
13 checks passed
@antond-weta antond-weta deleted the rawtoaces_data branch July 21, 2025 04:33
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.

4 participants