Skip to content

Conversation

ReeceHumphreys
Copy link
Contributor

@ReeceHumphreys ReeceHumphreys commented Aug 20, 2025

  • Tickets addressed: bsk-1072
  • Review: By file
  • Merge strategy: Merge (no squash)

Description

This PR aims to simplify and refactor our CI pipeline. By using GitHub actions we can have reusable components in the CI jobs that are easier to reason about and will be useful for automating the deployment of wheels in the future. The goals of this PR were to generally avoid changing CI behavior unless it removed duplication or increased consistency.

Notable changes

  • Created .github/actions which contains a reusable build action for bsk and the docs
  • Updated pull-request-closed.yml to merge.yml and simplified logic to reflect this is a workflow only intended to run on merges
  • Simplified pull-request.yml by using the reusable actions
  • Simplified version bump checking
  • Gated docs build/deploy behind successful macOS builds

Verification

This PR will trigger workflows to validate the updates. Future commits may be added to resolve issues found during this testing. The PR should be squashed and merged to avoid polluting the git history.

Documentation

N/A

Future work

  1. Evaluate which platforms to support, which Python versions to test on each, and whether to include Conan-based installs. This is out of scope for this PR, which focuses only on refactoring and simplifying the existing CI.

  2. Update the CI to automatically create and deploy wheels to PyPI for all supported platforms. We currently build and store wheels for ubuntu but do not publish them.

  3. Simplify the various requirements.txt files and move them into pyproject.toml per PEP 518 and PEP 621

Closes #1072

@ReeceHumphreys ReeceHumphreys self-assigned this Aug 20, 2025
@ReeceHumphreys ReeceHumphreys added enhancement New feature or request ci Continuous integration refactor Clean up with no new functionality labels Aug 20, 2025
@ReeceHumphreys ReeceHumphreys force-pushed the feature/bsk-1072-github-workflow-refactor branch from 6362117 to cc8c323 Compare August 20, 2025 18:12
@ReeceHumphreys ReeceHumphreys moved this to 🏗 In progress in Basilisk Aug 20, 2025
@ReeceHumphreys ReeceHumphreys force-pushed the feature/bsk-1072-github-workflow-refactor branch 9 times, most recently from 875e8e5 to 8b14def Compare August 28, 2025 17:30
@ReeceHumphreys ReeceHumphreys marked this pull request as ready for review August 29, 2025 18:56
@ReeceHumphreys ReeceHumphreys requested a review from a team as a code owner August 29, 2025 18:56
@ReeceHumphreys ReeceHumphreys requested a review from schaubh August 29, 2025 19:11
Copy link
Contributor

@schaubh schaubh left a comment

Choose a reason for hiding this comment

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

Nice refactor. Just a few comments and a question about including the one new python requirement to BSK requirements, rather than just including it in the CI test builds?

@ReeceHumphreys ReeceHumphreys moved this from 🏗 In progress to 👀 In review in Basilisk Sep 1, 2025
@ReeceHumphreys ReeceHumphreys force-pushed the feature/bsk-1072-github-workflow-refactor branch 2 times, most recently from 1b5888f to 5c4b56b Compare September 1, 2025 01:57
@schaubh
Copy link
Contributor

schaubh commented Sep 1, 2025

In notice we have 10 "pending check" where the PR is waiting for the results for the old CI test run names. When this is pushed, I'll have to remember to edit the repo settings to look for these new test run names. Do you have GitHub access privileges to look into this yourself? Obviously not before this branch is pushed.

Copy link
Contributor

@schaubh schaubh left a comment

Choose a reason for hiding this comment

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

love this branch. I really like how you have refactors the CI scripts. Just some very minor items and questions about the commit history. You add some stuff in the first commit, only to un-add it in the 2nd commit. Should be easy to clean up.

Also, when pushed to develop, we'll need to change the GitHub repo settings to make the PR requirements look for these new CI test names.

@ReeceHumphreys ReeceHumphreys force-pushed the feature/bsk-1072-github-workflow-refactor branch from 5c4b56b to aaeae8d Compare September 1, 2025 16:20
@ReeceHumphreys ReeceHumphreys force-pushed the feature/bsk-1072-github-workflow-refactor branch from aaeae8d to faf14e1 Compare September 1, 2025 16:25
@ReeceHumphreys
Copy link
Contributor Author

In notice we have 10 "pending check" where the PR is waiting for the results for the old CI test run names. When this is pushed, I'll have to remember to edit the repo settings to look for these new test run names. Do you have GitHub access privileges to look into this yourself? Obviously not before this branch is pushed.

I don't believe so. I think the only GitHub settings privilege I have is to add new Repository secrets for actions.

@ReeceHumphreys ReeceHumphreys force-pushed the feature/bsk-1072-github-workflow-refactor branch from f8e01fc to 9ef547c Compare September 1, 2025 21:55
ReeceHumphreys and others added 3 commits September 1, 2025 16:01
Update GitHub Actions to build and test only the latest supported
Python on Windows and Linux. On macOS, test all supported Python
versions.
Astro constants should be imported from Basilisk.architecture.astroConstants.
The duplicated values in src/utilities/astroFunctions.py have been deprecated for a year
and are now removed.
@ReeceHumphreys ReeceHumphreys force-pushed the feature/bsk-1072-github-workflow-refactor branch from 9ef547c to 9d0b878 Compare September 1, 2025 22:01
@ReeceHumphreys
Copy link
Contributor Author

Commit 985a42a was updated to add an extra macOS build with the --vizInterface False added to the conan_args.

@ReeceHumphreys ReeceHumphreys merged commit da97bde into develop Sep 2, 2025
22 of 28 checks passed
@ReeceHumphreys ReeceHumphreys deleted the feature/bsk-1072-github-workflow-refactor branch September 2, 2025 01:54
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Basilisk Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous integration enhancement New feature or request refactor Clean up with no new functionality
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Refactor CI/CD Pipeline
2 participants