docs: open the upgrade guide with require, not update - #74
Merged
Conversation
The first command could not do what the guide asked of it. Consumers pin a major — ^4.0, ^5.0 and ^5.1.1 across the three checked — and `composer update itk-dev/openid-connect-bundle` will not cross a major, so it reports nothing to do and leaves the reader on 5.x wondering why none of the guide applies. Nothing in the document mentioned ^6.0 at all. Found by walking the guide again after the restructure, this time following it literally rather than installing the bundle from a path repository, which had hidden the step. The 4.x hop gets the same treatment: `composer require` for both packages rather than `composer update`, which was where the partial-update refusal came from in the first place.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #74 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 177 177
===========================================
Files 14 14
Lines 732 732
===========================================
Hits 732 732
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
A second coherence pass over
UPGRADE-6.0.md, walked against two consumers again now that #71 restructured it. The structure held up; the very first command did not.The finding
The guide opened with:
That cannot install 6.0. Every consumer pins a major, and
composer updatewill not cross one:devops_itksites^5.1.1economics^4.0display-api-service^5.0So the reader runs it, composer reports nothing to do, and they are left on 5.x wondering why none of the guide applies.
grep '\^6\.0' UPGRADE-6.0.mdreturned nothing — the target version appeared nowhere in the document.It now opens with
composer require itk-dev/openid-connect-bundle:^6.0and says why. The 4.x hop gets the same treatment, which is also where the partial-update refusal comes from.Why the earlier pass missed it: I validated by pointing
composer.jsonat a path repository, which replaces the constraint outright. That is a fine way to test the bundle and a poor way to test the guide. This time I followed the document literally.The rest of the guide checks out
Both consumers walked end to end against
develop:economics(4.2.0 → 6.0,developfast-forwarded to remote first): the documented 4.x refusal reproduced verbatim, and after naming both packages it needs no configuration change at all — the optional expiry date in #71 removed the one key it previously had to add. Container compiles in dev and prod, 431 tests and 19,246 assertions pass, callback path derives as/openid-connect/generic, and the unset expiry reportsunknown.devops_itksites(5.1.1 → 6.0, with its expiry key removed to match the pre-6.0 shape): compiles in dev and prod, 52 tests pass, nothing to change.Steps 2 and 3 are no-ops for both: neither catches
AuthenticationExceptionaround the callback nor overridesonAuthenticationFailure(), and neither references the removed exceptions.Both repositories were restored afterwards — economics to
4.2.0on its original branch with containers stopped as found, itksites to5.1.1.