Skip to content

Initial CreateRun and set status.#279

Merged
talex5 merged 1 commit intoocurrent:masterfrom
tmcgilchrist:check_run
Jul 22, 2021
Merged

Initial CreateRun and set status.#279
talex5 merged 1 commit intoocurrent:masterfrom
tmcgilchrist:check_run

Conversation

@tmcgilchrist
Copy link
Copy Markdown
Member

Sample run on this PR tmcgilchrist/ocaml-bitbucket#19

Should eventually be an alternative for setting the status against a commit.

@tmcgilchrist tmcgilchrist marked this pull request as ready for review July 15, 2021 06:32
Copy link
Copy Markdown
Contributor

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

Looks good! (minor comments inline)

What happened to the plan to add a type parameter to Api.t, to enforce the rule that checks can only be used with apps?

Looks like ocaml/opam-repository#19046 is taking a long time to get merged, but we can go ahead with the submodule to get this merged for now.

let name t =
match String.split_on_char '/' t.owner_name with
| [owner;repo_name] -> Some (owner, repo_name)
| _ -> None
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.

Can this happen?

Copy link
Copy Markdown
Member Author

@tmcgilchrist tmcgilchrist Jul 20, 2021

Choose a reason for hiding this comment

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

It seems we always have both and this can't really fail in practice, the refactoring looks like this
5da9b03

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.

That looks good to me (with old code removed rather than commented).

@tmcgilchrist
Copy link
Copy Markdown
Member Author

What happened to the plan to add a type parameter to Api.t, to enforce the rule that checks can only be used with apps?

I like the idea it just started to get quite invasive and I abandoned it.

Looks like ocaml/opam-repository#19046 is taking a long time to get merged, but we can go ahead with the submodule to get this merged for now.

Agree, I'll clean this up and merge.

Copy link
Copy Markdown
Contributor

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

Looks good! I guess the ocaml-github vendoring should be removed now that the release is complete.

@tmcgilchrist tmcgilchrist force-pushed the check_run branch 2 times, most recently from d3b412e to 1af648c Compare July 22, 2021 00:38
Refactor owner_name into split owner / repo pair.
@talex5 talex5 merged commit 4c05890 into ocurrent:master Jul 22, 2021
@talex5
Copy link
Copy Markdown
Contributor

talex5 commented Jul 22, 2021

Thanks!

tmcgilchrist added a commit to tmcgilchrist/opam-repository that referenced this pull request Apr 7, 2022
…, current_github, current_git, current_examples, current_docker and current (0.6)

CHANGES:

Core:

- Implement labelling of clusters on the Graphviz diagram
  (@ewanmellor ocurrent/ocurrent#255)

- Abort propagation on constant/equal changes (@art-w ocurrent/ocurrent#318)

API:

- GitHub: Record build status using CheckRun (@tmcgilchrist ocurrent/ocurrent#279)

- GitHub: Add details_url to check_run. (@tmcgilchrist ocurrent/ocurrent#282)

- GitHub: Add Current_github.Api.cmdliner_opt to allow writing
  pipelines which can optionally be run as GitHub apps. (@talex5 ocurrent/ocurrent#281)

- GitHub: Provide markdown details for CheckRun. (@tmcgilchrist ocurrent/ocurrent#288)

- GitHub: Fix wrong name used for repository (@tmcgilchrist ocurrent/ocurrent#289 ocurrent/ocurrent#290)

- GitHub: Support Github rebuild via webooks. (@tmcgilchrist ocurrent/ocurrent#283)

- GitHub: monitor GraphQL queries (@art-w ocurrent/ocurrent#298)

- GitHub: Limit CheckRunStatus summary and text fields to 65535.
  (@tmcgilchrist ocurrent/ocurrent#300)

- GitHub: Log extra context for Webhook validation failure.
  (@tmcgilchrist ocurrent/ocurrent#302)

- GitLab: Initial GitLab plugin work. (@tmcgilchrist ocurrent/ocurrent#299)

- Git: Make git reset less verbose (@kit-ty-kate ocurrent/ocurrent#293)

Web UI:

- Use Lwt.pause instead of Lwt_unix.yield (@MisterDA ocurrent/ocurrent#297)

- Use `ansi` instead of `current_ansi` (@samoht ocurrent/ocurrent#321)

- Show line numbers and allow jumping to specific lines in job
  logs (@punchagan ocurrent/ocurrent#309)

Docker:

- Explicitly set confirmation levels to allow for
  manually triggered jobs. (@tmcgilchrist ocurrent/ocurrent#304)

- Stop using `Dockerfile.t` completely and use strings instead.
  (@MisterDA ocurrent/ocurrent#301 ocurrent/ocurrent#316)

Other:

- Update to cohttp 4.0.0 (ocurrent/ocurrent#274, @talex5)

- Move `Current_incr` to its own repository (ocurrent/ocurrent#284, @talex5)
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