Repository navigation
Conversation
KiteUtils 0.14 only reverses the sign of azimuth_east, which KiteGeometry never calls. Both tracked manifests move KiteUtils alone to 0.14.0; the 1.12 one also picks up JSONSchema and URIs, which the test environment already declared but that manifest was missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1-Bort-1
commented
Oct 22, 2026
1-Bort-1
left a comment
Member
Author
There was a problem hiding this comment.
Independent review (advisory)
Verdict: APPROVE WITH COMMENTS · 1 inline, 0 off the diff
Good
- The compat change is minimal and matches the card:
Project.tomlandtest/Project.tomlboth go from0.13.1to0.13.1, 0.14, so the package and its tests share one bound. - The claim that KiteGeometry never uses the changed azimuth holds: its only KiteUtils imports are
Logger, Settings, SysLog, save_log, wrap2pi(src/KiteGeometry.jl:18), and a grep finds no KiteUtils azimuth call insrcortest. - The manifest changes stay narrow: only KiteUtils moves to 0.14.0 in both pinned environments. The JSONSchema and URIs additions to the 1.12 manifest are explained in the card, since test/Project.toml already declares JSONSchema.
- The CHANGELOG entry is one compact line under
## Unreleasedand does not touch the version, which fits §5 and §9. - Leaving transform azimuth as east-towards-south and asking about it on SymbolicAWEModels.jl#430 keeps this PR to one idea and routes the semantic question to a human, as §1 and §5 ask.
Not good
Manifest-v1.12.toml.default:368— Moving both pinned manifests to 0.14.0 means no CI job checks the 0.13.1 floor still advertised in Project.toml:20. A regression against 0.13 would ship unnoticed to the very dependents that compat range is kept for.- The card doesn't mention that both manifests change the KiteGeometry entry from
version = "0.1.0"to0.3.2. It is a correct resync with Project.toml, but it is an unexplained line in the diff. - With both pinned manifests on 0.14.0, CI no longer tests the 0.13.1 lower bound the compat still claims. Nothing in this PR shows that 0.13 keeps working.
- The card says KiteUtils 0.14 changes only the azimuth sign, but gives no source (release notes or a compare link).
wrap2pi,save_logandSysLogare used here and are trusted on that claim alone. - The 1.12 manifest was never tested against 0.14.0, as the card itself notes, so the merge depends on the GitHub CI result still pending.
claude, rubric CLEAN_CODE.md. A different lab from the implementer
on purpose: a reviewer sharing its blind spots would not flag its mistakes.
Member
Author
|
Local full suite: PASS (2 min, Julia 1.13.0, one cell of the matrix) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1-Bort-1
marked this pull request as ready for review
October 22, 2026 16:18
This branch has not been deployed
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
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.
KiteGeometry accepted only KiteUtils 0.13, so no package built on it could move to KiteUtils 0.14. SymbolicAWEModels needs to (OpenSourceAWE/SymbolicAWEModels.jl#430). KiteUtils 0.14 changes one thing: the sign of its east-referenced azimuth (v0.14.0 release notes, OpenSourceAWE/KiteUtils.jl#165). KiteGeometry never uses that function, so it now accepts either version. Keeping 0.13 lets dependents move one at a time.
The pinned environments this repository installs from now use KiteUtils 0.14.0, so CI tests against the new version. Under them,
Pkg.resolvealone would have kept KiteUtils on 0.13, so I updated KiteUtils by itself and left every other pin where it was. Both environments also record KiteGeometry's own version as 0.3.2, matching the package; they still said 0.1.0. The Julia 1.12 environment also gains JSONSchema and URIs. The test environment already declared JSONSchema, but that environment was missing it, and URIs comes with it. The Julia 1.13 environment already had both.With both pins on 0.14, CI no longer runs the 0.13.1 floor. This change touches no source code, so main's CI on 0.13.1 still covers it, and I ran the tests on 0.13.1 against this branch as well (below). A CI job that tests the lowest allowed versions would keep that covered for good; that is a change of its own, not part of this one.
The azimuth input of a transform still turns from east towards south, as before. Whether it should follow KiteUtils' new sense is asked on OpenSourceAWE/SymbolicAWEModels.jl#430 and is left out of this change. A patch release follows once this merges.
Verification
Pkg.update("KiteUtils")reached 0.13.3, the newest 0.13)test-log,test-transforms,test-authoring,test-structure), Julia 1.13: 847/847 pass on KiteUtils 0.14.0, and 847/847 pass on KiteUtils 0.13.1 (main's pin, this branch's code)Opened by
1-Bort-1, an AI agent working for @1-Bart-1.Closes #77 · task
KiteGeometry.jl-77