Support for CI status (addresses #92)

[sgpthomas]

This PR is still a work in progress. Everything is subject to change.

This PR adds initial support for reporting workflow statuses. At the moment, I have only implemented basic support for the Github workflow API. Support for other forges may make more sense in a separate pull request.

Todo

• Runs (essentially subtasks of a workflow)
  • Store run name + status in database
• Display workflow information in PR details page
• Change what we use as workflow id from commit + name to something else. maybe the API node id?

Changes

I've added workflow to the database schema which stores information about workflow runs. This bumps the database schema version up to 10. I like keeping the workflow table detached from the pullreq table because it makes it easier in the future to add support for reporting per-commit workflow statuses.

At the moment, workflow information is downloaded alongside pull request information. I only pull information about the most head-rev commit in the PR. This is done with a separate query to the API for now. It should be merged with the pull request query eventually, but this necessitates a change to ghub.

Workflow information is displayed alongside pull requests in the main view. This works by querying the database for workflow runs that have the same commit as the head-rev of the PR.

Logs

I've decided to post-pone handling logs for now. I think the PR is useful enough in it's current state, and don't think that I need to burden this PR with additional complexity.

[tarsius]

I haven't looked too closely yet, but this generally looks like you are on the right track.

[tarsius]

The status should probably be parsed once and be stored in the database, as I did for issues and pull-requests, I believe.

[tarsius]

I apologize for not getting around to reviewing this yet.

Most likely I won't get around to it next week, because I'll focus to land a contribution of my own elsewhere, which I have been working on for a few years now.

I've actually looked at this a bit two days ago, but I concentrated on superficial things, to make the real review a bit easier, once I get around to that. I have pushed the result of that to your branch now.

So far you have added new commits to fix issues in older commits and such. Feel free to continue to do that. (Alternatively if you decide to squash, feel free to squash my cosmetic changes into your commit as well.) Keeping broken and/or incomplete changes around as separate commits can sometimes be useful during development, but we always squash such feature branches before merge, because we don't merge known broken commits, not alongside other commits that fix them (doing that would make bisecting harder). If keeping these separate commits around does not actually help you, then it would be better to periodically squash.

Not saying that this should not be split into multiple commits eventually, maybe along the lines fetch-data | make basic use of data | make everything pretty. But splitting like that would probably make things harder for you, so if we decide that is needed, then that can be delayed until all the other work is done.

Hm, on second thought, it would make things a lot easier for me, so please consider doing it now. (If you do that, and you discover, for example, a bug in the "fetch data" commit, which is followed by other commits, then you should create a fixup commit, that fixes that commit. You can use c F to immediately rebase to squash that change. At other times it might make sense to keep the fixup around as a commit in its own right for a while, by using c f and delaying the squashing until a later time.

At the moment, workflow information is downloaded alongside pull request information. I only pull information about the most head-rev commit in the PR. This is done with a separate query to the API for now. It should be merged with the pull request query eventually, but this necessitates a change to ghub.

From a quick look at forge--fetch-workflow, I got the impression that it needs some work. I don't think you need node and ... on PullRequest. If you want to work on something, while you wait for my review, I would suggest extending ghub-fetch-repository to include workflow information.

Based on that you might want to look into writing something like ghub-fetch-pullreq (notice how it transforms the query in ghub-fetch-repository) but is limited to just workflow data. That could be useful if it turns out that running ghub-fetch-repository just to check for updated workflow state is annoyingly slow.

Going in another direction, it could also be useful to fetch updated workflow state for all repositories in the local database at once, similar to how notifications are fetched.

[from #92] It appears that I will have to make changes to the ghub graph ql queries in order to get information about check suites and workflows. I can of course make this into a separate query and use the ghub-graphql function, but then this adds an additional query to the API. I was wondering if there is a particular reason that these queries live in ghub and not forge.

Originally we fetched less data, essentially "what everyone would need", so it made sense to keep that in ghub. Over time this was extended to account for forge's need for more data (which also led to the creation of ghub-fetch-repository-sparse, to fetch only the most essential data).

The question whether that should be moved to forge is valid, but I think it is better to keep this in ghub. By keeping it there, we are forced to keep it free of things that depend on forge. Yes, we might extend to query to account for the needs of forge, even if those additions are not of interest to any other (potential) consumers, but keeping the code there is still useful: worst case scenario is that someone has to copy-paste it from there and then strip the unneeded parts, but doing that is much easier if they do not also have to strip out code that depends on things defined in forge, and write replacements for those parts.

[ezemtsov]

@tarsius is there any chance we could merge it?
This functionality would be super cool for our monorepo with dynamically generated check list 😸

[sgpthomas]

I got busy with work and haven't finished the PR yet. Have some more time finally and so will turn my attention towards finishing it.

[sgpthomas]

Is doing the ghub-fetch-pullreq thing more efficient or better than using ... on XXX? My limited understanding is that you can use it to fetch less information by starting the query at some particular node in the graph. It seems like the ghub-fetch-pullreq approach creates queries that always start from the top-level repository node. I'm happy to use that approach, it's just not immediately obvious to me why it's better.

[tarsius]

Is doing the ghub-fetch-pullreq thing more efficient or better than using ... on XXX?

That are really do separate questions. My point was that the query can be written without the ... on X syntax:

(query
 (repository
  (pullRequests
   [(:edges t)
    (:singular pullRequest number)
    (orderBy ((field UPDATED_AT) (direction DESC)))]
   number
   id
   (commits   [(:edges 10)]
              (commit
               oid
               (checkSuites [(:edges t)]
                            id
                            databaseId
                            conclusion
                            status
                            resourcePath
                            url
                            (workflowRun
                             id
                             databaseId
                             runNumber
                             (workflow name))
                            (checkRuns [(:edges t)]
                                       id
                                       databaseId
                                       name
                                       conclusion
                                       status
                                       permalink
                                       )))))))

---

I'll get back to this once I have landed main...notif, which should be in about a week. Switching between the two would be painful, as they both make modifications to the database.

[tarsius]

I still plan to implement this kind of functionality, but I do not intend to base it on this pull-request, so I am closing it.

I won't be working on this soon, but now that Forge 0.4 and Magit 4.0 are finally out, I can finally turn my attention to this kind of new functionality. However, this is not the only such addition that has been requested, and it's still not the one I am prioritizing.