exercism / exercism/configlet

lint: Consider the design of Exercism-wide checks

Open
#178 1 comment 0 reactions 0 assignees View on GitHub
cmd: lint kind: design
Dominant language
Nim
Stars
23
Forks
17
Avg merge
13h 57m
Merged PRs (30d)
1

Description

### Background
- It's convenient for Exercism to store track configuration on GitHub, and sync with the website after a PR gets merged. This makes it easy for people to contribute, and we've done it like this for a long time.
- It's convenient for Exercism to have a separate repo for each track.
- We want to check that tracks are correctly configured, so we have a tool called `configlet` with a `lint` command that runs some checks during CI.
- Some of the linting rules involve checking every track. For example, we want to check that a PR on one track repo does not add a UUID that already exists on Exercism. And that for a concept exercise `config.json`: the value of `forked_from` refers to an implemented exercise on another track

### Ways to implement Exercism-wide checks
1. Don't implement any Exercism-wide checks in `configlet`. This means that we increase the problems that are found only at sync-time, rather than CI-time. But it's possible to automate the opening of a GitHub issue if a problem is found at sync-time, and we might want to do that anyway - there will always be race conditions. For example: simultaneously merging two PRs that add the same UUID might cause `configlet lint` to pass on both during CI, but one of the commits could be rejected at sync-time.
2. Implement Exercism-wide checks in `configlet`, but make them offline. It's straightforward to bake some information into the binary, such as a big list of UUIDs (maybe 100 KiB when compressed), and implemented exercises. The information would be updated upon each `configlet` release, and would probably stay current enough to catch the majority of problems. We fallback to opening GitHub issues if problems are later caught at sync-time.
3. Implement Exercism-wide checks in `configlet`, and make them online. Some implementations:
- a) Just naively download the `config.json` file for every track from GitHub. That involves ~80 requests every time `configlet lint` runs, which is rather wasteful, but can still be fast. We might run into rate-limiting issues, though.
- b) Download a special file that contains the `config.json` file for every track, which is kept in-sync with the website. It could be one JSON file, or a tarball of all the `config.json` files. This is like `3a`, but uses only one request.
- c) Download a special file that contains the all the information required for online checks, which is kept in-sync with the website, containing e.g. a full list of UUIDs. This is like `3b`, but with a smaller file.
- d) Build an Exercism API endpoint, and make requests to it. For example: we could send it a list of UUIDs, and the API returns the list of UUIDs that are invalid/duplicate, etc. This should produce the smallest network transfer, but might not necessarily be faster - especially if we have to make multiple requests.

If we do option 3, some design questions are:

#### When to run the online checks?
`configlet lint` could run them:
- By default
- By default, but respect an `--offline` flag
- When `configlet` detects it is running in CI
- When `configlet` detects it is running in CI, or if an opt-in flag is passed
- Only if a flag is passed (but this is more fragile - it requires adding a flag to every track's `configlet` workflow)

#### Use `curl`, or the Nim stdlib?
Advantages of just using `curl`:
- Easier support for concurrent downloads. If we concurrently download every track's `config.json` file, a GitHub Action step that does that takes something like 2 seconds. If we used the stdlib we'd want to make `async` requests and have a connection pool, which adds some complexity.
- Smaller binary size. We don't have to compile with `-d:ssl`, and think about whether to statically link OpenSSL/LibreSSL. `async` requests also would add some bloat.
- Supports gzip during requests by default. Although Nim's stdlib doesn't do this yet, the Nimble package [`guzba/zippy`](https://github.com/guzba/zippy) is excellent and easy to use (see [example](https://github.com/guzba/zippy/blob/master/examples/http_client.nim)). [`treeform/puppy`](https://github.com/treeform/puppy) is also nice.

Disadvantages:
- We could feel more pushed towards making the online checks of `configlet lint` opt-in or run in CI only. Otherwise we'd have to also add a Windows implementation, and check that the installed `curl` is recent enough.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.