fix: unbreak main, and add the CI that would have caught it - #9
Merged
Conversation
main does not compile. #7 and #8 both edited the test module in src/config.rs from branches that did not know about each other. Git merged them cleanly — the blocks are textually distinct — but #7's test references `Identity::PnmSession`, an enum variant #8 deleted. A textual merge cannot see that; only building the result can. Nothing built the result. This repo has shipped eight PRs with no CI at all, only a DCO check, so a main that fails `cargo test` merged and sat there. The library and binary still build, which is why it was quiet: the breakage is confined to the test target, so anyone cloning to *use* the plugin would not notice, and anyone cloning to *work* on it would trip over it immediately with no idea it was pre-existing. Removes the stale test — #8 already covers its intent — and adds CI: - fmt, clippy and test under RUSTFLAGS=-D warnings. - On **macOS**, deliberately. Two of the bugs shipped here were platform-specific paths (`dirs::config_dir()` is ~/Library/Application Support, not ~/.config), and a Linux-only runner would have been green through both. - On `merge_group` as well as `pull_request`, because a PR green on its own branch is exactly what just broke main when combined with another. - A manifest job: the plugin JSON files and the command/skill frontmatter are what a person meets before any Rust runs, and a malformed one breaks the plugin without breaking the build. Every step was dry-run locally before being committed, since a CI file that has never executed is a guess. Signed-off-by: Glenn Gore <glenn.gore@gmail.com>
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.
maindoes not compile.#7 and #8 both edited the test module in
src/config.rs, from branches that did not know about each other. Git merged them cleanly — the blocks are textually distinct — but #7's test referencesIdentity::PnmSession, an enum variant #8 deleted. A textual merge cannot see that; only building the result can.Nothing built the result. This repo has shipped eight PRs with no CI at all, only a DCO check.
It stayed quiet because the breakage is confined to the test target:
cargo buildstill succeeds, so anyone cloning to use the plugin would not notice, and anyone cloning to work on it would trip over it immediately with no way to tell it was pre-existing rather than their own doing.The fix
Removes the stale test — #8 already covers its intent with
the_sessions_dir_is_never_left_to_a_default— and adds CI.The CI, and why it looks like this
RUSTFLAGS=-D warnings.pnmstores config underdirs::config_dir(), which is~/Library/Application Supporton macOS and~/.configon Linux. A Linux-only runner would have been green through both.merge_groupas well aspull_request, because "a PR that was green on its own branch" is exactly what just brokemain. Checking only the branch would not have caught this.plugin.jsonbreaks the plugin without breaking the build, so the build alone would never report it.Every step was dry-run locally before being committed — a CI file that has never executed is a guess, and this PR is about not guessing.
Testing
cargo test— 84 pass.cargo clippy --all-targets --all-featuresclean under-D warnings.cargo fmt --checkclean. Both CI jobs' commands run locally.