Skip to content
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.

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

Add option to run bump after update #11942

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

carlos-granados
Copy link

@carlos-granados carlos-granados commented Apr 22, 2024

  • Adds a new bump-after-update config parameter
  • Adds a new bump-after-update option to the update command

If the bump-after-update config parameter is set or the bump-after-update option is passed to the update command, it will run the bump command after finishing the update. This command will only be run if the update finishes successfully

If the config parameter or the option are set to dev or no-dev then only the corresponding dependencies will be bumped.

The dry-run option passed to the update command will also be used for the bump command

If any packages are listed in the update command, this same package list will be used by the bump command.

Includes updates to the documentation and new tests

Fixes #11906

@carlos-granados carlos-granados force-pushed the add-bump-after-update-option branch 2 times, most recently from 6903fb0 to cc20ff5 Compare April 22, 2024 12:11
@carlos-granados
Copy link
Author

I can see that the PHP 8.4 tests are failing but the failures seem unrelated to these changes and they also seem to happen in the main branch

@carlos-granados carlos-granados force-pushed the add-bump-after-update-option branch 2 times, most recently from 0d8312d to 0d9f1b9 Compare April 22, 2024 14:03
{
$this->initTempComposer($composerJson);

if ($createLock) {
Copy link
Author

Choose a reason for hiding this comment

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

The bump command needs that a lock file exists, so when testing the bump-after-update version we need to create one

Copy link
Member

@Seldaek Seldaek left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, after a quick review just one comment but I'll need to take a closer look when I'm back at my desk :)

doc/06-config.md Outdated
@@ -476,4 +476,9 @@ throw, but you can set this config option to `["example.org"]` to allow using sv
URLs on that hostname. This is a better/safer alternative to disabling `secure-http`
altogether.

## bump-after-update

Defaults to false. If set to true, Composer will run the Bump command after running
Copy link
Member

Choose a reason for hiding this comment

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

I think this could be a string|bool that also accepts "no-dev" and "dev" as values to configure the *-only flags.

Copy link
Author

Choose a reason for hiding this comment

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

This suggestion sounds good. I'll wait until you come back and are able to take a deeper look. If you still feel this is the way to go, I'll modify the PR

Copy link
Member

Choose a reason for hiding this comment

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

Yes please :) Definitely feel like this would be an improvement.

bool $noDevOnly,
bool $dryRun,
array $packagesFilter,
bool $calledFromUpdate = false
Copy link
Member

Choose a reason for hiding this comment

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

Seems unused and unnecessary?


if ($bumpAfterUpdate) {
if ($result === 0) {
$io->writeError('<info>Running Bump after Update.</info>');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$io->writeError('<info>Running Bump after Update.</info>');
$io->writeError('<info>Bumping dependencies</info>');

Comment on lines 270 to 271
} else {
$io->writeError('<warning>Not running Bump after Update because the update command did not finish successfully.</warning>');
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is needed.

Suggested change
} else {
$io->writeError('<warning>Not running Bump after Update because the update command did not finish successfully.</warning>');

@Seldaek Seldaek added this to the 2.8 milestone Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants