This should solve #318.
Add documentation for Forgejo CI Actions runner #369
@linusg Hey, just to be sure, it is OK for us / the PR author to reuse material from your blog (incl. screenshots) under the terms of the license of this repository (with attribution in the commit information and a small thank you note), correct? We are not sure how much of it we'll use yet, but it won't be substantial.
(We would also very much appreciate your input on this PR!)
Yes, absolutely fine :)
Just keep in mind that a few things have changed since I wrote that (at least I had to tweak my hosted one at least once), so some information may be outdated. I'll take a proper look at this PR later 👍
This is a great PR, and especially a great first PR.
I didn't execute the instructions to confirm their accuracy because of my lack of experience in that sector, which is super important for a "step-by-step"-esque tutorial. There are also remaining TODOs and I would appreciate a second opinion; feel free to contact me again and I'll try to make the time to take a much more careful look if this does not happen soon after you complete those TODOs.
But language- and style-wise, everything is pretty great, proven by the fact that I can't really go beyond "grasping at straws" in a document of this size - and, apart from one style issue, the only "actual" problems are purely cosmetic does not concern instructions, as in, prevent the user from doing what they need to do. I would even merge it as it is, as this goes beyond the standards of what we normally expect.
As this is the perspective I normally review PRs from, I am submitting a preliminary approval, even if this is not 100% ready to merge. If everything else (accuracy of instructions, screenshot) are good, I'm fine with merging this as-is.
Overall, great job, and thank you.
@ -0,0 +97,4 @@
## Deployment Methods
You can deploy the runner in different environments, depending on your current setup and how much you care about security.
Content-wise, this is a nice "bridge" / introduction, but it's kind of leaving me confused. Maybe this sentence should be rewritten. "in different environments" is confusing me a bit. Rephrasing "how much you care about security" to "security needs" may also be better.
"how much you care about security" is a bit disorienting, as it insinuates that the infrastructure may be more prone to compromise if the wrong decision is taken, without much guidance as to how to achieve an optimal setup.
Here's two examples that I would build on top of:
Depending on your setup and your security needs, there is a wide variety of options for deploying [...]
There are various methods for deploying a Runner, choosing the right one depends on your setup and your security needs.
@ -0,0 +115,4 @@
{% endadmonition %}
You will need to get the GID of docker in your host machine by running:
Why? Also, it would be cool if you could consistently capitalize names of tools like "docker" -> "Docker".
@ -0,0 +253,4 @@
mountPath: /certs
```
Make sure to replace the value of `token` with your registration token.
This could work as an admonition. (warning)
(totally didn't forget the fact that the new page has to be added to the main CI page and had it brought by the OP)
b638601467
to 6ade771206
Thank you for the great feedback and kind words. I made the appropriate amendments, and added two screenshots.
Nice work! Left a couple of comments, but for the most part this LGTM :)
@ -0,0 +5,4 @@
parent: CI
---
Recently, a GitHub Actions-compatible feature has been integrated in Forgejo,
Would probably rewrite this to drop Recently; it's not relevant here.
@ -0,0 +41,4 @@
<picture>
<source srcset="/assets/images/ci/actions/create-new-runner.webp" type="image/webp">
<img src="/assets/images/ci/actions/create-new-runner.png" alt="Create new Runner">
Nit: different indentation
@ -0,0 +51,4 @@
- Download the binary to run on your machine:
```bash
nit: the correct language tag for this is console: https://stackoverflow.com/a/49004070
@linusg That is not true in the syntax highlighter or markdown formatter the documentation is using. Applying console instead of bash breaks the syntax highlighting.
bash is valid even in the given context (GitHub Markdown format), you'll find the citations if you scroll in the replies or other answers in the page you linked.
It is valid, sure, but semantically not correct :)
I don't know what highlighter the docs use, but here on Forgejo console yields better results (first is console, second is bash):
$ command
output
$ another command
$ command
output
$ another command
@ -0,0 +60,4 @@
- Or download the container image and run it using Docker:
```bash
likewise
@ -0,0 +75,4 @@
Binary:
```bash
likewise
@ -0,0 +77,4 @@
```bash
$ ./forgejo-runner register --no-interactive --token {TOKEN} --name {NAME} --instance https://codeberg.org
$ INFO Registering runner, arch=amd64, os=linux, version=3.2.0.
I doubt the $ here are part of the output? looks like a prompt, I'd definitely avoid that.
@ -0,0 +85,4 @@
Docker:
```bash
likewise
@ -0,0 +113,4 @@
Podman can work by executing:
```bash
likewise (I'll let you find the rest yourself :))
@ -0,0 +152,4 @@
image: docker:dind
privileged: true
volumes:
- certs:/certs
nit: indentation is four spaces here
@ -0,0 +293,4 @@
<picture>
<source srcset="/assets/images/ci/actions/demo-workflow.webp" type="image/webp">
<img src="/assets/images/ci/actions/demo-workflow.png" alt="Demo workflow running successfully">
nit: indentation
if you show me up again like that it's a ban!!!!
jk, i should still have paid more attention to the indentation stuff as i deliberately glossed over the code blocks / html tags and just read the text; thank you very much for the help @linusg :D
6ade771206
to 9b4e246ea2
@linusg Thank you for the feedback!
PR scope expanded by request of the author, not to be merged yet
b82fcb4541
to 508543c9bc
508543c9bc
to c1ae67e041
c1ae67e041
to 2f54328b7f
@n0toose should be good to merge!
Thanks for your tenacity and for your contributions, again, awesome work! ❤️
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?