Update data storage format from annoying Markdown table rows, and automate updating README.md#137
Merged
Update data storage format from annoying Markdown table rows, and automate updating README.md#137
Conversation
andrlime
changed the title
andrlime
force-pushed
the
automations
branch
2 times, most recently
from
3dd7cac to
79613e2
Compare
Member
|
huge. |
andrlime
commented
Jul 28, 2025
egelja
requested changes
Jul 28, 2025
Member
egelja
left a comment
egelja
left a comment
There was a problem hiding this comment.
Great work! This is sick. Two small things.
| name: "[ci] Lint and compile source code" | ||
|
|
||
| on: | ||
| pull_request: |
Member
There was a problem hiding this comment.
Id recommend only doing the git push on main, but running it on PR or push to main. It’s confusing seeing commits pushed on a PR. You can upload the generated readme to artifacts and link to it in a comment in the PR for preview functionality.
Also, this should only run when files in data/ are changed.
Member
Author
There was a problem hiding this comment.
- On push, commits stuff. On PR, adds a comment with a link to a zipped artifact. A concern here I have is, we have branch protections on main. If we "push to main" via a merge, is the bot able to push to a protected branch? If not, I'd say pushing to the PR is still cleaner.
- Yup, added artifacts and comments
- Added a check for the data/ folder
Member
There was a problem hiding this comment.
andrlime
force-pushed
the
automations
branch
2 times, most recently
from
d19f7ad to
b635058
Compare
Contributor
Contributor
Contributor
Contributor
glasss13
approved these changes
Sep 5, 2025
andrlime
force-pushed
the
automations
branch
from
b3e383a to
c11b2d6
Compare
Contributor
Contributor
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR automates updating the README.md file with YAML data files, and an OCaml parser that uses an existing ppx library. Here is why this change is good, on balance:
"blah blah"is easier than manually looking for which part of the Markdown table to update which is prone to error.Here is why this PR is dubious/bad
1. Needs some work on the OCaml side of things - that's to say, updating OCaml structs isn't exactly user friendly. The idea here is to take YAML files that contain the same info and just parse it into the structs. That should resolve the issue pretty quickly.Fixed via YAML parsing2. GitHub action takes a few minutes to run because even though it caches dependencies, recompiles them or something. At most five minutes though, not too big of an issue. If this is too slow, someone can just run manually, force push, and merge.Takes two minutes with caching