docs: name only the relup task for Castle - #34
Merged
Conversation
forecastle#24 renamed one task and the README claimed it had renamed two: "Both are named for `Castle`" took in the appup compiler, which is `Mix.Tasks.Compile.Appup` and is invoked as `mix compile.appup` through the project's `:compilers` list. There is no `mix castle.appup` to reach. The relup task's `@moduledoc` carried the same implication by saying it is named `castle.*` "like the rest of the tooling a developer runs". Castle's README states the corrected rule as of castle#9, so left alone the two halves of one release would have given a reader contradictory naming rules for the same compiler. The RELEASE.md entry from forecastle#24 needs nothing: it names only `mix forecastle.relup` as renamed and already says `mix compile.appup` is unaffected because it is named by its `:compilers` entry rather than by a package. This correction and that entry are both unreleased 1.0.0 notes, so this is a correction to what will ship rather than a change to it. Refs: #24, ausimian/castle#9 Claude-Session: https://claude.ai/code/session_01RotroiBdbidiCRX3KqE3Dy
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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.
Corrects an error introduced by #24. Found by the adversarial review on the Castle side of the same work (ausimian/castle#9).
The problem
#24 renamed one task and the README claimed it had renamed two:
Only the relup task is named for Castle. The compiler is
Mix.Tasks.Compile.Appup, invoked asmix compile.appupthrough the project's:compilerslist, and there is nomix castle.appupfor a reader to go looking for. The relup task's@moduledoccarried the same implication, saying it is namedcastle.*"like the rest of the tooling a developer runs".Castle's README states the corrected rule as of ausimian/castle#9, so left alone the two halves of one release would have given a reader contradictory naming rules for the same compiler.
What changed
README.md— the paragraph now namesmix castle.relupas the thing named for Castle, and states separately that the compiler is named for neither package: it ismix compile.appup, reached through:compilersrather than invoked by name, so there is nomix castle.appup.lib/mix/tasks/castle.relup.ex— the@moduledocprovenance line no longer implies the rest of the tooling shares the namespace.Documentation only; no code paths touched.
Deliberately not done
No RELEASE.md entry. The breaking entry from #24 is already precise — it names only
mix forecastle.relupas renamed and saysmix compile.appupis unaffected because it is named by its:compilersentry rather than by a package. That entry and this correction are both unreleased 1.0.0 notes, so this corrects what will ship rather than changing it. The reviewer independently reached the same conclusion.Verification
mix precommitgreen: 353 tests, 0 failures, credo --strict clean.Mix.Tasks.Compile.Appup(compile.appup) andMix.Tasks.Castle.Relup(castle.relup) with nocastle.appup, that a repository-wide search finds no remaining claim that the compiler is Castle-named, and that the Installation and Appup Compiler sections consistently configure:appupthrough:compilers.https://claude.ai/code/session_01RotroiBdbidiCRX3KqE3Dy