switch from github workflows to github actions workflows; test [githubactionsworkflowstatus githubworkflowstatus] #8475

chris48s · 2022-10-03T20:51:15Z

Refs #8146
Refs #4587
This PR implements the solution suggested in #8146 (comment) - deprecate the old route and make a new one.

shields-ci · 2022-10-03T20:52:52Z

	Messages
📖	✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 7e7d006

services/github/github-actions-workflow-status.service.js

chris48s · 2022-10-03T20:53:07Z

services/github/github-workflow-status.service.js

-  static category = 'build'
-
-  static route = {
+export default deprecatedService({


Normally when we deprecate a badge we are just getting rid of it completely. Often the upstream service has been retired, or whatever. If we can provide a migration path from old to new, we use a redirect. In this case, we can't do either of those things because the :workflow param is not compatible between the two badges. Additionally, the GitHub workflow status badge is one of our most popular badges 😬 I think deprecating the old route and making people switch is the right thing to do because the behaviour of the existing badge is unexpected but its going to generate some support requests/anger/confusion/etc. Before we merge this I definitely want to write up a pinned issue (I've not written it yet, but I will) explaining what is going on and how to migrate.

Because we've never had this situation where we're asking people to switch and we can't provide a redirect, we've never done this before, but I wonder if instead of serving build | no longer available we make the issue and serve build | github.com/badges/shields/issues/9999 (also use the link property for any <object> embeds) to help direct people towards upgrade advice. I'm somewhat conflicted as it is mainly info for maintainers and most of the people who will see it are going to be end users who can't do anything about it.

but I wonder if instead of serving build | no longer available we make the issue and serve build | github.com//issues/9999 (also use the link property for any <object> embeds) to help direct people towards upgrade advice. I'm somewhat conflicted as it is mainly info for maintainers and most of the people who will see it are going to be end users who can't do anything about it.

I'd recommend going with your issue link suggestion. Completely agreed that 99% of those that see the badge won't be able to do anything about it, but having a no longer available message instead doesn't really improve their experience one way or another and I think having the issue link only increases awareness; some subset of those users that see the issue-link badge in someone else's project will have their own such badges in their respective repos that they do need to take action on but may still be unaware

OK. I have made a placeholder issue #8671 so we have a known URL we can link to, and I've done that in de39617 . This should allow us to approve the code changes. I will work on writing up the issue explaining why we are making the change, what users need to do about it, etc before I deploy it.

calebcartwright · 2022-10-08T02:08:09Z

Acknowledge seeing this one, processing the proposed options

chris48s · 2022-10-17T15:18:00Z

In light of #8398 and #4587 I've pushed 7ea7cf5 which updates the implementation of this to use the GitHub API instead of scraping the badge. This has the advantage it will allow self-hosting users to use it for private repos.

chris48s · 2022-10-17T15:18:35Z

services/github/github-actions-workflow-status.service.js

+          page: '1',
+          per_page: '1',
+          exclude_pull_requests: 'true',


These params are here for performance reasons. There's huge execution time difference between this query with exclude_pull_requests=true (fast) vs exclude_pull_requests=false (slow)

Interesting. Definitely understand the performance angle, but curious if there's any functional impacts? Am I correctly interpreting this as meaning that runs triggered by a pull request would be excluded?

Nope. Passing this means the pull_requests array in the JSON response is not populated, but we aren't using any values from it so it doesn't really matter here. We can still get the details of a workflow that is triggered by a PR. For example, if I call https://api.github.com/repos/badges/shields/actions/workflows/enforce-dependency-review.yml/runs?branch=frontend-gha&exclude_pull_requests=true (the enforce-dependency-review.yml workflow on our repo only runs on PR triggers), we get a response with all the info we need for a badge. Just the pull_requests array is empty.

chris48s · 2022-10-17T15:19:49Z

services/github/github-actions-workflow-status.service.js

+
+const queryParamSchema = Joi.object({
+  event: Joi.string(),
+  branch: Joi.string().required(),


I've made branch a required param for this badge. Not my favourite solution, but workflow can contain a / and branch=HEAD doesn't work so we can't make the default "whatever the default branch is" in one request, so required query param it is. Lets lay that marker down while we're making a breaking change.

I don't think "latest workflow status across all branches" is a sensible default.

Do you know off hand if this has to explicitly be a branch or will the upstream endpoint accept any valid ref?

The docs say

branch string

Returns workflow runs associated with a branch. Use the name of the branch of the push.

Anecdotally, I tried some calls with a tag and a commit hash (which I expected would return a valid result if this could accept any ref). They returned 0 results.

chris48s · 2022-12-01T19:56:08Z

services/github/github-workflow-status.service.js

+      We've implemented it as a custom class instead of a normal
+      deprecatedService so that we can include link.
+      */
+      link: ['https://github.com/badges/shields/issues/8671'],


Because deprecatedService() throws an exception, we can't customise it beyond the label/message so I've done this as a custom class. I think the number of cases where this will be actually clickable will be low but it is worth doing IMO. This is a sufficiently exceptional edge-case that I don't mind this being a bit special and wierd and custom.

chris48s · 2022-12-04T13:57:58Z

Here is a draft of the issue text go to in #8671

🥱 TL;DR

If you have a github workflow test.yml

name: Run Tests
...

You need to update your badge URL from

https://img.shields.io/github/workflow/status/<user>/<repo>/Run%20Tests
to
http://img.shields.io/github/actions/workflow/status/<user>/<repo>/test.yml?branch=main

There are two changes here:

The workflow parameter is now the workflow filename, not the name: defined in yaml
Branch is now a required query param

❓ Why are you making this change?

This change resolves issue the problem reported in #8146 After consulting with GitHub support, it transpired that identifying workflows using the name in the yaml file can be ambiguous in some situations and defaults to finding workflows on any branch when a branch param is not specified, rather than the default branch. Switching to identifying workflows by the workflow filename and making branch a required parameter resolves these problems. Two workflows can have the same name string in yaml, but filename is unique on a given branch.

We decided that dropping the old badges (using the workflow name) and requiring users to switch to the new URLs (rather than keeping the old ones "working") was the right choice for our users because in some cases the old badges were serving data which was inaccurate or unexpected in subtle ways. Serving accurate data is the most important thing for us to prioritise.

😭 You made a breaking change! Waa

We know. We try never to do this and it is not a decision we took lightly, especially given this is one of our highest traffic badges. In general, our philosophy is: If you embedded a shields badge in your README in 2013 and the upstream service is still online, that badge should still work today. We achieve this with almost no exceptions.

Occasionally we do need to change badge routes for various reasons, but usually we keep old URLs working using redirects. In this case, it was not possible to provide a transition because the new badge route uses a different parameter.

One of the reasons we try to never do this is because we don't really have a great way to communicate a breaking change to our users or provide warning before it happens. We have no idea who uses our service. We don't ask anyone to sign up, we don't track you. Hopefully given that constraint we have done a reasonable job of this.

📚 Further Reading

calebcartwright · 2022-12-14T17:46:18Z

Couple thoughts on the text from #8475 (comment)

The workflow parameter is now the workflow filename, not the name: defined in yaml

Since the file can be placed in a subdirectory, I think it would be worth including an extra line in the issue to (hopefully) cover that preemptively. Something simple along the lines of the below should help IMO:

If your workflow file resides in a subdirectory, then be sure to include the subdirectory path along with the file name, e.g.

http://img.shields.io/github/actions/workflow/status/<user>/<repo>/sub_directory/test.yml?branch=main

question Why are you making this change?

I'd recommend we open with a summary of #8146 since most people won't read and digest. At the root of all this was that GitHub essentially silently abandoned the endpoint we were using which semi-broke our badge that relied on that endpoint.

It might then be worth underscoring this in the final section too; our hand was forced and we were left with no real alternative options

chris48s · 2022-12-14T20:36:01Z

Thanks - good suggestions. I'll do another draft of the issue text and deploy this in the next few days.

switch from github workflows to github actions workflows

808dfec

chris48s added the service-badge New or updated service badge label Oct 3, 2022

chris48s changed the title ~~switch from github workflows to github actions workflows~~ switch from github workflows to github actions workflows; test [githubactionsworkflowstatus githubworkflowstatus] Oct 3, 2022

chris48s commented Oct 3, 2022

View reviewed changes

chris48s mentioned this pull request Oct 14, 2022

Switch GitHub Workflow Status Badge to API #4587

Closed

update github actions workflow badge to use api

7ea7cf5

chris48s commented Oct 17, 2022

View reviewed changes

add test case for missing branch param

de39617

This was referenced Oct 24, 2022

Support '/' in Github Workflows name #4559

Closed

Update documentation for GitHub Workflow Status Badges #7102

Closed

custom deprecation message

e0b7dbd

chris48s commented Dec 1, 2022

View reviewed changes

calebcartwright approved these changes Dec 14, 2022

View reviewed changes

Merge branch 'master' into 8146-gitlab-workflows

7e7d006

chris48s merged commit 61dd6c0 into badges:master Dec 15, 2022

This was referenced Dec 15, 2022

Change to GitHub workflow badge routes #8671

Closed

Correct the fetch URL for Github Actions workflow status badge #8310

Closed

chris48s mentioned this pull request May 21, 2023

Support GitHub workflow badge for private repos #9189

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

switch from github workflows to github actions workflows; test [githubactionsworkflowstatus githubworkflowstatus] #8475

switch from github workflows to github actions workflows; test [githubactionsworkflowstatus githubworkflowstatus] #8475

chris48s commented Oct 3, 2022 •

edited

Loading

shields-ci commented Oct 3, 2022 •

edited

Loading

chris48s Oct 3, 2022

calebcartwright Nov 30, 2022 •

edited by chris48s

Loading

chris48s Dec 1, 2022

calebcartwright commented Oct 8, 2022

chris48s commented Oct 17, 2022

chris48s Oct 17, 2022

calebcartwright Nov 30, 2022

chris48s Dec 1, 2022

chris48s Oct 17, 2022

calebcartwright Nov 30, 2022

chris48s Dec 1, 2022

chris48s Dec 1, 2022

chris48s commented Dec 4, 2022

calebcartwright commented Dec 14, 2022

chris48s commented Dec 14, 2022 •

edited

Loading

switch from github workflows to github actions workflows; test [githubactionsworkflowstatus githubworkflowstatus] #8475

switch from github workflows to github actions workflows; test [githubactionsworkflowstatus githubworkflowstatus] #8475

Conversation

chris48s commented Oct 3, 2022 • edited Loading

shields-ci commented Oct 3, 2022 • edited Loading

Choose a reason for hiding this comment

calebcartwright Nov 30, 2022 • edited by chris48s Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calebcartwright commented Oct 8, 2022

chris48s commented Oct 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chris48s commented Dec 4, 2022

🥱 TL;DR

❓ Why are you making this change?

😭 You made a breaking change! Waa

📚 Further Reading

calebcartwright commented Dec 14, 2022

chris48s commented Dec 14, 2022 • edited Loading

chris48s commented Oct 3, 2022 •

edited

Loading

shields-ci commented Oct 3, 2022 •

edited

Loading

calebcartwright Nov 30, 2022 •

edited by chris48s

Loading

chris48s commented Dec 14, 2022 •

edited

Loading