Skip to content

Conversation

grdsdev
Copy link
Contributor

@grdsdev grdsdev commented Sep 17, 2025

Summary

  • Split CI/CD into separate workflows for better separation of concerns and security
  • Enhanced CI workflow with uv caching and proper dependency management
  • Added dedicated release workflow with proper release-please integration
  • Fixed conditional publishing logic and added pre-publish validation

Changes Made

CI Workflow Improvements (.github/workflows/ci.yml)

  • Focused on testing only: Removed release/publish jobs for cleaner separation
  • Enhanced uv caching: Added enable-cache: true and cache-dependency-glob: "uv.lock"
  • Explicit dependencies: Added uv sync --all-extras --dev for proper workspace setup
  • Simplified permissions: Only contents: read needed for CI operations

New Release Workflow (.github/workflows/release.yml)

  • Proper conditional logic: Uses release_created output instead of unreliable commit message parsing
  • Safe publishing: Only runs when release-please actually creates a release
  • Pre-publish validation: Runs linting and full test suite before publishing
  • Correct checkout: Uses specific release tag instead of potentially stale HEAD
  • Enhanced security: Minimal required permissions for each job
  • PyPI trusted publishing: Properly configured with id-token: write

Documentation (CLAUDE.md)

  • Comprehensive development guide: Commands, architecture, and best practices
  • Monorepo structure: Clear explanation of workspace organization
  • Common commands: Make targets for testing, linting, building, and infrastructure
  • Package-specific notes: Special considerations for each package

Test Plan

  • Verify CI workflow runs tests correctly with new uv caching
  • Ensure release workflow only triggers when release-please creates releases
  • Confirm pre-publish checks run before PyPI publishing
  • Validate CLAUDE.md provides clear development guidance

🤖 Generated with Claude Code

grdsdev and others added 2 commits September 17, 2025 12:58
- Split CI/CD into separate workflows for better separation of concerns
- Enhanced CI workflow with uv caching and explicit dependency installation
- Added proper release workflow using release-please outputs
- Fixed conditional publishing logic to use release_created output
- Added pre-publish validation steps (linting and testing)
- Created CLAUDE.md with comprehensive development guidance

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Fix missing newline at end of release.yml file

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@grdsdev grdsdev requested a review from o-santi September 17, 2025 16:07
@coveralls
Copy link

Pull Request Test Coverage Report for Build 17803671610

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.74%

Totals Coverage Status
Change from base Build 17801447810: 0.0%
Covered Lines: 8610
Relevant Lines: 9185

💛 - Coveralls

Copy link
Contributor

@o-santi o-santi left a comment

Choose a reason for hiding this comment

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

I don't think I'll be merging this. Splitting it into two files doesn't add anything, its just creating another file for putting the exact same jobs there. There's no need for uv sync between running tests, the commands inside makefile already manage that correctly. I don't think we should enable caching in uv, as caching in CI its the most common source of bugs and uv is by far the fastest part of our setup. It is also missing id-token: true in the setup for publishing.

Addittionally, (I might be wrongly interpreting this but I do believe that) its setting up the CI to publish on every open version PR, instead of just when the main PR is merged, which is not what we want.

I also oppose to adding a CLAUDE.md file. Firstly, it is specific to the claude model, and if we add it, anyone with a different model will try to add its own version for its own file. Secondly, its a concatenation of all the READMEs of all the packages, no additional information there. Thirdly, its going to be unmaintained doc pretty fast, specially if it spreads to other other models.

Fourth, I don't think AI is going to be of great help for most of the work that is needed in this repository, as its not really able to understand some of the core details in these CI setups -- eg. the fact that its trying to set it up to publish when the PR is created, which is not correct.

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.

3 participants