Skip to content

Add support for --minimal-changes with full updates #12349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
Seldaek merged 2 commits into from
May 13, 2025
Merged

Add support for --minimal-changes with full updates #12349

Seldaek merged 2 commits into from
May 13, 2025
+69 −5

Conversation

@Seldaek
Copy link
Member

@Seldaek Seldaek commented Mar 24, 2025

Refs #12341

@Seldaek Seldaek added this to the 2.9 milestone Mar 24, 2025
@Seldaek Seldaek requested a review from naderman March 24, 2025 12:53
@naderman
Copy link
Member

naderman commented Mar 24, 2025

Looks good, just wonder if there should be an error/warning if --minimal-changes is used on a full update and the composer.json is unmodified, which will never result in any modifications?

@Seldaek
Copy link
Member Author

Seldaek commented Mar 24, 2025

Well.. you ask for minimal updates you get minimal updates :p

stof
stof reviewed Mar 24, 2025
View reviewed changes
tests/Composer/Test/Fixtures/installer/full-update-minimal-changes.test
{ "name": "dependency/pkg", "version": "2.0.0" },
{ "name": "dependency/pkg", "version": "1.1.0" },
{ "name": "dependency/pkg", "version": "1.0.0" },
{ "name": "dependency/pkg2", "version": "2.0.0" },
Copy link
Contributor

@stof stof Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest having both 2.0.0 and 2.1.0 (which would both match the requirement), to cover that --minimal-changes will still update to 2.1.0 and not to 2.0.0 (i.e. it is not the --minimal-deviation that was suggested in the discussion)

Copy link
Member Author

@Seldaek Seldaek Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 12221a3 (I think that's what you meant anyway)

Copy link
Contributor

@stof stof Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

@stof stof mentioned this pull request Apr 4, 2025
Open
@Seldaek Seldaek merged commit 6b55179 into composer:main May 13, 2025
20 checks passed
@Seldaek Seldaek deleted the full_update_minimal branch May 13, 2025 12:20
@dkarlovi
Copy link

dkarlovi commented May 13, 2025

IIUC this update: if you didn't update composer.json by say requesting some version bump, it will not update anything? And if you did update say 1.0 to 1.1, it will only bump stuff required for that change (which might be nothing)?

In that case I do agree with @naderman about unmodified composer.json producing an error, it will help with UX since it makes clear to users nothing will happen unless they make something happen.

@Seldaek
Copy link
Member Author

Seldaek commented May 14, 2025

Yes, I'll see if we can easily add a warning or not.

Seldaek added a commit to Seldaek/composer that referenced this pull request May 23, 2025
@Seldaek
Add warning when --minimal-changes is used without package and a fres…
b8b1a2d 
…h lock file

Refs composer#12349
@Seldaek Seldaek mentioned this pull request May 23, 2025
Merged
Seldaek added a commit that referenced this pull request May 23, 2025
@Seldaek
Add warning when --minimal-changes is used without package and a fres…
30b7660 
…h lock file (#12418)

* Add warning when --minimal-changes is used without package and a fresh lock file

Refs #12349
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@naderman naderman naderman approved these changes

+1 more reviewer

@stof stof stof left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.9

Development

Successfully merging this pull request may close these issues.

4 participants

@Seldaek @naderman @dkarlovi @stof