Conversation
acolombier
left a comment
There was a problem hiding this comment.
I had an initial look and this is already looking very promising! Just waiting to see if we can sort the explicit prettier before testing locally :)
|
@acolombier Thanks for the nice review. I applied the suggested changes with some fixes for the workflow (git commands inside repo, switch & stage before commit, env vars for push step). If I'm right the next step is to add Mixxxbot token |
|
@acolombier I have now also got both pre-commits (manual & Mixxx) working for me, but it seems that hooks:
- id: prettier
- types: [yaml]
+ files: \.(d\.ts|ts|tsx|js|yaml)$ |
acolombier
left a comment
There was a problem hiding this comment.
If I'm right the next step is to add Mixxxbot token MIXXXBOT_TS_TYPES_AUTOUPDATER_PAT to the secrets (I copied and renamed it from the changelog workflow before) and then this should work in theory?
That's correct! For security reasons, repo secrets are not accessible in fork PR, since it is very easy to exfiltrate them! So you should make the commit & push steps conditional so it doesn't trigger if the secret is not available, like we do in the main repo (note that you need to set a global env to the secret, then apply your condition to the env itself, as doing it directly to the secrets fails due to a bug)
I have now also got both pre-commits (manual & Mixxx) working for me, but it seems that mixxx-controls.d.ts is not formatted automatically. The reason for this is the prettier hook config in .pre-commit-config.yaml. It only formats yaml files. By adding this it works and the mixxxx-controls.d.ts is formatted correctly (I had to change types to files and regex, because types: [yaml, ts] didn't work):
Perfect! Note that the pre-commit we need to change is the one from mixxxdj/mixxx, since this is where mixxx-controls.d.ts lives, but I think this is already what you did :)
Done, added condition and set MIXXXBOT_TS_TYPES_AUTOUPDATER_PAT as env var.
Yes. I will make a pull request into the Mixxx repo for that. In addition, I now run the pre-commit again beforehand and stage the change, because otherwise the commit would theoretically fail. |
|
@acolombier The job runs now! I did some fixes, merged my group descriptions PR from 2.5 and added some missing stem group descriptions, because in my script I check that every group has a description. Regarding formatting, we still have the problem that, as I see it, there is no standard formatting for |
1a62f04 to
998be76
Compare
Hi,
as discussed in this pull request for the Mixxx repository here is my suggestion for how we can generate the TypeScript Mixxx control types from the manual.
This pull request includes:
build/.doctrees/chapters/appendix/mixxx_controls.doctreeand generatesbuild/mixxx-controls.d.tsBtw., I have no idea if the workflow can function this way because I am not really familiar with GitHub Actions. The workflow is open for discussion.