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

Upgrade to 7.0.5 #109

Merged
merged 12 commits into from
Jan 23, 2025
Merged

Upgrade to 7.0.5 #109

merged 12 commits into from
Jan 23, 2025

Conversation

hawkyre
Copy link
Contributor

@hawkyre hawkyre commented Jan 20, 2025

Fixes https://github.com/doofinder/support/issues/3171

Upgrade library version to 7.0.5

After this, the package needs to be updated in https://packagist.org/packages/doofinder/doofinder

@hawkyre hawkyre requested review from JoeZ99 and teoortuno January 20, 2025 15:25
@hawkyre hawkyre self-assigned this Jan 20, 2025
@hawkyre
Copy link
Contributor Author

hawkyre commented Jan 20, 2025

I have no idea why it's failing if I haven't changed the lockfile at all

@teoortuno
Copy link
Contributor

I have no idea why it's failing if I haven't changed the lockfile at all

Maybe that's the problem. Look at the logs of CI:

# Lock file errors
- The lock file is not up to date with the latest changes in composer.json, it is recommended that you run `composer update` or `composer update <package name>`.

Copy link
Member

@JoeZ99 JoeZ99 left a comment

Choose a reason for hiding this comment

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

I have no idea why it's failing if I haven't changed the lockfile at all

have you tried what they suggest in the "Details" of each failed run?

@JoeZ99 JoeZ99 self-requested a review January 21, 2025 08:24
composer.lock Outdated
@@ -823,50 +926,46 @@
},
{
"name": "phpunit/phpunit",
"version": "5.7.27",
"version": "5.5.4",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this downgrading?

composer.lock Outdated
@@ -1207,21 +1306,21 @@
},
{
"name": "sebastian/exporter",
"version": "2.0.0",
"version": "1.2.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is downgrading too.

composer.lock Outdated
@@ -1230,7 +1329,7 @@
"type": "library",
"extra": {
"branch-alias": {
"dev-master": "2.0.x-dev"
"dev-master": "1.3.x-dev"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also downgrading.

composer.lock Outdated
},
{
"name": "symfony/yaml",
"version": "v4.4.45",
"version": "v3.4.47",
Copy link
Contributor

Choose a reason for hiding this comment

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

And this.

@hawkyre hawkyre requested a review from teoortuno January 21, 2025 09:46
Copy link
Contributor

@teoortuno teoortuno left a comment

Choose a reason for hiding this comment

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

Look at this branch: master...refs/heads/teo/test

All I have done is:

  1. Do the changes in CHANGELOG.md
  2. Update the version in composer.json file
  3. Run composer update doofinder/php-doofinder

That's it. No need to change CI flow. All checks passed.

Comment on lines 22 to 23
- name: Update composer dependencies
run: composer update
Copy link
Contributor

Choose a reason for hiding this comment

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

With this, the composer validate is validating the composer.lock that is modified by composer update, not the composer.lock that is in git. So it's not doing the proper validation. There will be failures checking the content hash of the lock file.

There's no need to do a composer update every time in CI. We do a composer install later already.

@hawkyre
Copy link
Contributor Author

hawkyre commented Jan 21, 2025

@teoortuno Of course, the old CI does NOT use the PHP versions it says it does. It was broken since it was created. Take a look at it, you'll see nowhere it sets the PHP version other than the CI flow title.

@hawkyre
Copy link
Contributor Author

hawkyre commented Jan 21, 2025

Obviously it will work, it's just using the same version for every flow and it's the same version as the one used to generate the lockfile.

@teoortuno
Copy link
Contributor

@teoortuno Of course, the old CI does NOT use the PHP versions it says it does. It was broken since it was created. Take a look at it, you'll see nowhere it sets the PHP version other than the CI flow title.

D'oh! Yeah, good catch.

But I still think that the composer validate must be made before the composer update.

@hawkyre
Copy link
Contributor Author

hawkyre commented Jan 21, 2025

So we can just do validate then update?

@hawkyre
Copy link
Contributor Author

hawkyre commented Jan 21, 2025

Let me see

@hawkyre
Copy link
Contributor Author

hawkyre commented Jan 21, 2025

It's failing now @teoortuno

@hawkyre
Copy link
Contributor Author

hawkyre commented Jan 21, 2025

With the same error that made me put composer update before validate actually

@teoortuno
Copy link
Contributor

With the same error that made me put composer update before validate actually

Run composer update doofinder/php-doofinder in your environment. That should update the content-hash in composer.lock file.

@hawkyre hawkyre merged commit 12c07d9 into master Jan 23, 2025
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants