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

Fix Smith CI #1279

Merged
merged 8 commits into from
Dec 5, 2024
Merged

Fix Smith CI #1279

merged 8 commits into from
Dec 5, 2024

Conversation

chapman39
Copy link
Collaborator

@chapman39 chapman39 commented Dec 3, 2024

Smith CI has been running only Serac tests due to an issue with using symlinks! This MR attempts to resolve this.

EDIT: Turns out the fix was within Smith. The remaining changes here are minor cleanups.

@chapman39 chapman39 added bug Something isn't working CI Continuous Integration labels Dec 3, 2024
@chapman39 chapman39 self-assigned this Dec 3, 2024
@chapman39 chapman39 changed the title Fix smith ci: run build script in smith repo Fix Smith CI Dec 3, 2024
@white238
Copy link
Member

white238 commented Dec 3, 2024

This being broken in this way actually has more implications than we think. Anywhere that get_repo_dir is called needs to be reviewed or that function needs to be fixed.

@white238
Copy link
Member

white238 commented Dec 3, 2024

Good catch btw....

@chapman39 chapman39 requested a review from white238 December 3, 2024 21:11
@chapman39
Copy link
Collaborator Author

The previous solution didn't solve the problem all the way, since there were issues with get_project_name() returning serac instead of smith.

This new solution modified get_repo_dir() to find the smith dir if it's a serac submodule. The only catch with this solution is that you won't be able to build and test only serac if it's a submodule of smith.

@chapman39 chapman39 merged commit 194b3ad into develop Dec 5, 2024
2 checks passed
@chapman39 chapman39 deleted the bugfix/chapman39/smith-ci branch December 5, 2024 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants