-
Notifications
You must be signed in to change notification settings - Fork 551
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
ci: implement release verification #1264
Conversation
src/scripts/ci.zig
Outdated
// TODO: these two don't work yet | ||
if (language == .go or language == .dotnet) continue; |
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.
Yes, these two are broken! Looking into fixing them, but want to do that independently from this work to add basic verification.
src/testing/tmp_tigerbeetle.zig
Outdated
_ = try shell.project_root.statFile(tigerbeetle_exe); | ||
}; | ||
// If tigerbeetle binary does not exist yet, build it. | ||
// TODO: just run `zig build run` unconditionally here, when that doesn't do spurious rebuilds. |
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.
@matklad tidy.zig complaining about line length here
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.
Should be fine now!
6b8af43
to
042098e
Compare
fix for go: #1265 |
See dotnet/sdk#12160 (comment) I don't entirely understand what's happening here, but adding this flag indeed results into native libraries included in the archive. This should be caught by #1264 in the future
and for .net! |
5a96d23
to
e2bc88d
Compare
5f4b90e
to
338be3f
Compare
src/clients/java/ci.zig
Outdated
\\ <dependency> | ||
\\ <groupId>com.tigerbeetle</groupId> | ||
\\ <artifactId>tigerbeetle-java</artifactId> | ||
\\ <version>0.14.158</version> |
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.
Shouldn't we use the targetted version here?
\\ <version>0.14.158</version> | |
\\ <version>{version}</version> |
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.
Most excellent catch, thanks!
Fun fact, when writing this code I've actually thought that options: struct
pattern has a drawback that it doesn't warn about unused field, as I made a similar mistake for some other language!
144379d
to
65aeeb2
Compare
This adds last-mile testing of the thing that was actually released (GitHub release, nmp package, etc). This is too late to _prevent_ issues, but helps to discover things otherwise impossible to discover (eg, some files were lost during upload to GitHub), and also helps with finding divergence of upstream dependencies (e.g., if something breaks on a new version of node js).
65aeeb2
to
4ccccb7
Compare
This adds last-mile testing of the thing that was actually released (GitHub release, nmp package, etc). This is too late to prevent issues, but helps to discover things otherwise impossible to discover (eg, some files were lost during upload to GitHub), and also helps with finding divergence of upstream dependencies (e.g., if something breaks on a new version of node js).