Skip to content

Cicd combined#95

Open
aviv1ron1 wants to merge 25 commits into
generative-computing:mainfrom
aviv1ron1:cicd-combined
Open

Cicd combined#95
aviv1ron1 wants to merge 25 commits into
generative-computing:mainfrom
aviv1ron1:cicd-combined

Conversation

@aviv1ron1

Copy link
Copy Markdown
Collaborator

No description provided.

aviv1ron1 added 11 commits July 9, 2026 15:22
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
@codecov-commenter

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

aviv1ron1 added 14 commits July 9, 2026 21:39
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Ruff loads its config from pyproject.toml, so a malformed pyproject.toml
makes ruff abort with a parser error instead of the clean check-toml
message. Running check-toml/check-yaml first (with fail_fast) ensures
syntax errors are reported by the dedicated validator, consistently
between local commits and CI's --all-files run.

Signed-off-by: aviv ron <rona@il.ibm.com>
check-added-large-files only inspects staged additions, so on CI's clean
checkout (nothing staged) it was a no-op. --enforce-all makes it check all
files under pre-commit run --all-files. uv.lock (~2.3 MB) is excluded as an
intentionally large lockfile.

Signed-off-by: aviv ron <rona@il.ibm.com>
check-merge-conflict returns early unless a merge is in progress, so on
CI's clean checkout it was a no-op. --assume-in-merge makes it always
scan, letting pre-commit run --all-files catch committed conflict markers.

Signed-off-by: aviv ron <rona@il.ibm.com>
…hooks

Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
uv lock resolves fine on macOS; only installing the vLLM/CUDA wheels
fails. --frozen is for not mutating the lock, not because a re-resolve
would fail. Also note the uv-lock pre-commit hook runs locally.

Signed-off-by: aviv ron <rona@il.ibm.com>
CI's coverage run executes only tests/unit/, which exercises just the
pure-Python config module. Measuring the whole granite_switch package
diluted the reported coverage to ~1%, because the hf/vllm/composer/
tutorials subpackages are GPU/model-only and unreachable in CI.

Add [tool.coverage.run] omit rules for those subpackages so the CI
coverage number reflects the code the unit suite is actually
responsible for (now ~96%). Full-project coverage continues to be
measured on the GPU cluster.

Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
Signed-off-by: aviv ron <rona@il.ibm.com>
@njs2017

njs2017 commented Jul 15, 2026

Copy link
Copy Markdown

PR Review

Strong infrastructure PR overall — the CI/CD direction looks good and the pre-commit/dev-experience improvements are valuable.

What looks good

  • Clear workflow split across CI, DCO, GPU tests, and publish
  • Good use of uv and caching
  • Nice contributor-experience improvements in CONTRIBUTING.md, CHANGELOG.md, and Makefile targets
  • README link fixes and .git-blame-ignore-revs are good cleanup

Requested changes

  1. DCO sign-offs
    The PR appears to include commits without DCO sign-off, so the DCO workflow will fail. Please re-sign the branch, e.g.:

    git rebase --signoff origin/main
  2. CI coverage gaps
    tests/composer/ and tests/hf/ are excluded from CPU CI, and GPU tests are manual-only. Please document the intended coverage model clearly in the PR / docs so reviewers know which paths are protected automatically vs manually.

  3. Custom hook maintenance
    .pre-commit/validate_links.py adds a fair amount of custom validation logic. It would help to add at least basic tests for:

    • ci/check_headers.py
    • ci/add_signoff.py
    • ci/check_dco.py
    • ideally the link/import validator too

Nits

  • There is some purely cosmetic churn (newline-only changes like .github/CODEOWNERS, .gitignore)
  • A few workflow commands differ in their use of --frozen; worth making that consistent where possible

Overall: good direction, but I’d request the DCO fix and clearer CI/test-coverage expectations before merge.

@aviv1ron1 aviv1ron1 marked this pull request as ready for review July 16, 2026 11:05
@aviv1ron1 aviv1ron1 requested a review from yairallouche as a code owner July 16, 2026 11:05
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