Skip to content

Fix README CI badge - #657

Open
JAB1305 wants to merge 2 commits into
eclipse-basyx:developfrom
rwth-iat:fix/readme-ci-badge
Open

JAB1305 wants to merge 2 commits into
eclipse-basyx:developfrom
rwth-iat:fix/readme-ci-badge

Conversation

@JAB1305

@JAB1305 JAB1305 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Update from old ci.yml to pr.yml to get badge to function again.

Also replaced with shields.io as github's badge would show up as "pr: passing" which might be confusing.
Instead shields badge to get "CI: passing".

Fixes: #656

Replace with shields.io as github's badge would show up as "pr: passing" which might be confusing.
Instead sheilds badge to get "CI: passing"

Fixes: eclipse-basyx#656

@zrgt zrgt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug (one):

  1. Badge counts PR runs as well as pushes. The URL filters only on branch=main. For a PR run, GitHub's branch filter matches the PR's source branch, so any PR opened from a branch named main counts, which is common for fork PRs. A failing fork PR from main would turn the badge red even when main itself is fine. This is already happening: in the live API, the newest pr.yml run with branch main is a pull_request run (2026-10-02 12:05), not the push at 11:59. That goes against issue #656, which wants the badge to show the last push to main.
    • Fix: add &event=push, giving pr.yml?branch=main&event=push&label=CI.

Minor:

  1. Link doesn't match the image. The image shows main only, but clicking it opens the full pr.yml run list for every branch. Point the link to actions/workflows/pr.yml?query=branch%3Amain+event%3Apush.
  2. shields.io isn't needed. The PR switches to shields.io only because GitHub's own badge reads "pr: passing". That text comes from the name: pr line in pr.yml. Renaming it to name: CI gives the GitHub badge the right label and avoids shields.io's caching delay and rate-limit outages.
  3. "CI" label is too broad. The badge covers only pr.yml. The separate main.yml workflow (ARM Docker build and smoke tests on PRs into main) isn't included, so the badge can say "CI: passing" while the release build is broken. Either use a narrower label like "tests" or add a second badge.

@JAB1305

JAB1305 commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback.

  1. and 2. I have addressed it in a small fix. For 3. and 4. I would propose to change

name: pr to name: CI within pr.yml

But I am not sure what would better describe pr.yml functionality than the term "CI". Else I will include a second badge regarding main.yml. There again the job might benefit of a better name than the current "main". Maybe pre-merge, smoke-test, arm-smoke? Open for suggestions.

@zrgt

zrgt commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Thanks — 1. and 2. look good.

On 3.: yes, name: pr → name: CI in pr.yml is what I meant. The rename only changes the workflow's display name — job names stay the same, so required status checks and branch protection are unaffected, and the badge path stays workflows/pr.yml. With that we can go back to the native badge and drop shields.io:

<a href="https://github.com/eclipse-basyx/basyx-python-sdk/actions/workflows/pr.yml?query=branch%3Amain+event%3Apush"><img src="https://github.com/eclipse-basyx/basyx-python-sdk/actions/workflows/pr.yml/badge.svg?branch=main&event=push" alt="CI"></a>

Both branch and event are supported by the native badge, see Adding a workflow status badge.

On 4.: let's drop the second-badge option — a badge for main.yml can't work. main.yml triggers only on pull_request: branches: [main], so every run is recorded under the PR's head branch, never under main: actions/workflows/main.yml/runs?branch=main returns total_count: 0, and the existing runs all have head_branch=develop (release PRs) or a topic branch. Since main is the default branch, a plain main.yml/badge.svg would permanently read "no status". The only filter that resolves anything is ?branch=develop&event=pull_request, which would label a release-PR run as the state of develop — more misleading than no badge at all.

So: one badge, pr.yml labelled CI. That holds up, because pr.yml is the gate for everything that lands on develop and main (tests, static analysis, docs, packaging, amd64 Docker smoke test); main.yml only adds the arm64 Docker smoke test on release PRs.

For the main.yml name: of your three I'd avoid pre-merge (pr.yml also runs pre-merge) and smoke-test (pr.yml has a Docker smoke test too, just amd64). arm-smoke is accurate on content; I'd slightly prefer release-gate, since it also says when it runs — the extra check before develop → main. Either is fine by me, and it's a follow-up rather than part of this PR, since the badge doesn't reference main.yml.

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.

2 participants