fix(generator): emit string macros as strings, not floats - #4
Merged
Conversation
The CD committed `public const float JOLT_VERSION = "5.5.0";` to main and then failed to
pack it. The macro classifier decided the type by looking at the value:
else if (value.EndsWith("f") || value.Contains("."))
type = "float";
A quoted string containing a dot satisfies that. JOLTC_JOLT_VERSION, which I added to
JoltPhysicsC yesterday so the binding could state which Jolt it wraps, is exactly that
shape -- so the change that made the version legible is the change that broke the build.
Two corrections. String macros are recognised before the numeric guesses, because a
quoted string can contain anything that looks like a number. And the float case now uses
TryParse instead of "contains a dot or ends in f", which accepted any value with
punctuation and emitted a float initialised with something that is not one.
`public const string JOLT_VERSION = "5.5.0"` is also the useful outcome: a consumer can
read the wrapped JoltPhysics version off the binding without chasing two repositories.
Verified by regenerating: the binding, the API tests and the sample all build.
Worth noting how this surfaced. CI passed on the pull request because it regenerates from
the *vendored* headers, which predate the version macro. The CD fetches from the tracked
release tag, which contains it. So the first run that could see this was the first run
after switching to release tracking -- and it was a dry run with publishing off, so
nothing shipped.
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
1 participant
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.
maincurrently does not compile. The CD committed this and then failed to pack it:Cause
The macro classifier decided the type from the value's shape:
A quoted string containing a dot satisfies that.
JOLTC_JOLT_VERSION "5.5.0"— which I added to JoltPhysicsC so the binding could state which Jolt it wraps — is exactly that shape. The change that made the version legible is the change that broke the build.Fix
TryParse. "Contains a dot or ends in f" accepted any value with punctuation in it and emitted a float declaration initialised with something that is not a float. The next such macro would have failed the same way.public const string JOLT_VERSION = "5.5.0"is also the outcome worth having: a consumer reads the wrapped JoltPhysics version off the binding, instead of chasing two repositories and resolving a commit against a tag list.Verified by regenerating — binding, API tests and sample all build.
Why CI did not catch it
CI regenerates from the vendored headers, which predate the version macro. The CD fetches from the tracked release tag, which contains it. So the first run that could see this was the first run after switching to release tracking.
That run was a dry run with
skip-assets-publishing, so nothing reached nuget.org. The cost was a redmainfor the length of this pull request.🤖 Generated with Claude Code