-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Not valid Github status badge for broken workflow #8146
Comments
Our badges for github workflows are basically just "scrapers" for github's own badges so when you call https://img.shields.io/github/workflow/status/Wandalen/wTools/mod_interface/alpha it calls https://github.com/Wandalen/wTools/workflows/mod_interface/badge.svg?branch=alpha and then reports the value from that. It is essentially just a "wrapper" that allows you to use the full range of shields styles. Looks like your builds are now fixed so I now can't reproduce it but if you can break one again, check the actual github badges and see what value you've got. |
Unfortunately I suspect this is reflective of some upstream changes GitHub have rolled out, relative to #7102 Here's what GitHub shows for some of your branches (screen grab at the bottom in case statuses change) It looks like GitHub updated the docs back in https://github.com/github/docs/pull/3802/files#diff-95d58394efa588c4f92f85274259efb8ca1e10a02bd346549e65b57604c156c4. However, I think that there's either a new(ish) issue in the original, by-name endpoint, or that GitHub have just recently rolled out the changes. As Chris noted we rely on the upstream providers to deliver information, which is what we then use. In this case the endpoint that we've always used for Workflow Status is claiming your master branch is passing, so that's what is being displayed. I think this is something that needs to be tracked down with GitHub to get a definitive answer. We can't simply "just switch" to the workflow file name model either as that would break our existing users which is not something we want to do if GitHub plan to fix the by-name endpoint. |
@calebcartwright What if we have it fetch legacy first and if it's a 404, fetch the new URL? (And an alternative to not use legacy at all for those with existing legacies that are now static) Say the word and I'll put it in my existing PR |
That's an interesting idea @exoRift. I think I could get behind that as a tactical solution (since the badges currently appear to be broken for half the audience), so long as we keep tracking towards something more long term. @chris48s would you have any concerns with this? I feel like it should be simple enough at the code level, and not like it would have any tangible impact on our token pool nor the traffic volume we throw github's way |
So if we think https://github.com/exoRift/react-fluent-mobile/workflows/Quality%20Assurance/badge.svg?branch=master is returning inaccurate or out-of-date info and I think trying to serve information we believe is wrong or outdated first is the wrong call. I think there are really 2 sensible things we can do: Option 1:
Option 2:Overload the existing route to take a generic
Option 2 has the advantage that all the existing badges out there in the wild will still "work", but if we think they are serving stale/inaccurate data then this is not necessarily much of an "advantage". I think serving an error saying "you need update your badge URL" is ultimately more useful then "here's a value that might not be right" I guess the thing I'd like to be slightly more sure of is: what is the status of the |
Good point. I'd already forgotten that the two endpoints need different inputs
Yup, I'd found that too and expressed some of the same thinking earlier in the thread. It's an odd scenario and I'm not sure the best way to get in touch with GitHub for a definitive answer on this particular item. Guess we can try the user forum |
I have a proposal for this issue:
Images example:Current behavior:Proposed new behavior:I am willing to do the PR too if this proposal eventually is approved 😉 |
Tbh, I'd rather just deprecate it rather than keep around a a "legacy" version if we can't rely on the data. I've started a discussion over at https://github.com/orgs/community/discussions/33739 to try and get an answer. If we don't get to the bottom of it there, I'll try contacting support. |
Nobody replied to https://github.com/orgs/community/discussions/33739 |
I got this message back from support, which is quite helpful and does explain what is happening:
I think my conclusion from this is that we should:
I'll submit a PR doing this next week (I'm going to be away for a few days), unless someone wants to jump on it over the weekend. |
See #8671 for the details of the fix for this |
|
Are you experiencing an issue with...
shields.io
🐞 Description
I noticed that
shields.io
provide status badgepassing
for broken workflowThe real results is :
The link to the workflow run on the screenshot. Repository where shields.io status badges are used.
The badge works well with valid workflow files. And this workflow is fixed now. Please, try to reproduce and fix the bug.
🔗 Link to the badge
https://img.shields.io/github/workflow/status/Wandalen/wTools/mod_interface?label=
💡 Possible Solution
No response
The text was updated successfully, but these errors were encountered: