-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
run go generate on CI #49
Conversation
CI fails because #46 is not yet merged. |
I'd be fully on board with this if code generation was deterministic... but it isn't - if just a few days or a week go by, there's a good chance the generated code will change. Plus, re-generation requires manual review, so I'm not sure that automation avoids human interaction either (see #50). We could make the generator fully deterministic across time by pinning a hash of the CSV, but then we just move the problem elsewhere - now the manual step is to update the SHA every few weeks or months :) I think there's a manual step every few months no matter what we do. Periodic checks/reminders could be useful, like a cron job - I'd personally prefer that over making commit CI builds start failing by no fault of their own. |
I am curious why the code generation is non-deterministic? |
Because it uses the multicodec CSV as input from its HEAD branch, and that CSV table is updated from time to time. |
Thinking outloud, in an ideal world this Go code generation would be in the same repo as where the CSV is, then we wouldn't have this complexity with separately-moving HEADs. The output could be a module in a sub-directory in the same root repository, for example. |
OK so when I hear non-deterministic; I think: Given the same CSV file would the generator generate the same code? And I think this is a "Yes". I'd then argue code generator is deterministic and should be automated. Want to manually release? fine. But I don't think a human looking at a code generated by running |
A note on HEAD: If a motivation to do things manually is "control" then the generator could be triggered when there is a tag on the repo that contains the CSV. That way we have two manual control points: first is manual tagging of CSV file, and second manual release of code generated automatically. |
Would it make sense to use a Git submodule here? That way things would be more predictable. |
At least to me, deterministic means "
I mean, sure, it can't hurt - but it's akin to pinning a commit SHA in the URL like I mentioned before, so you simply move "regularly re-run |
Maybe I misunderstood your last comment, actually. If you mean use a submodule to make |
That's what I meant. We won't be able to automatically update the submodule (unless we want to write an Action that does that and then opens a PR, but that's probably overkill), but at least we can enforce that the version of the submodule and the generated code are consistent. |
Yup that sounds good. Happy to review a PR if you want to take a crack. If we don't fancy submodules, we could always swap the raw github URL to use a commit hash instead of HEAD. |
8cb726d
to
c274c8d
Compare
If I remember correctly, other repositories also use submodules to include the multicodec repo. That's why we introduced the recursive submodule checkout in the Unified CI Setup, btw. I added the submodule and changed the code generator to read the file from there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Fixes #48.