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

chore: update psr/log #74

Merged
merged 5 commits into from
May 16, 2023
Merged

Conversation

kidager
Copy link
Contributor

@kidager kidager commented Jan 31, 2023

This PR

  • updates the psr/log version requirements

Notes

Some projects require other versions of psr/log than 1.*

Example:
On a fresh Laravel 9 project, running composer require open-feature/sdk yields the following error:

Info from https://repo.packagist.org: #StandWithUkraine
./composer.json has been updated
Running composer update open-feature/sdk
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires open-feature/sdk * -> satisfiable by open-feature/sdk[0.0.2, ..., 0.1.1, 1.0.0, ..., 1.3.1].
    - open-feature/sdk[0.0.2, ..., 0.1.1, 1.0.0, ..., 1.3.1] require psr/log ^1.1 -> found psr/log[1.1.0, ..., 1.1.4] but the package is fixed to 3.0.0 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.
You can also try re-running composer require with an explicit version constraint, e.g. "composer require open-feature/sdk:*" to figure out if any version is installable, or "composer require open-feature/sdk:^2.1" if you know which you need.

Installation failed, reverting ./composer.json and ./composer.lock to their original content.

@kidager kidager requested a review from tcarrio as a code owner January 31, 2023 18:09
@kidager kidager force-pushed the fix/psr-log-support branch from 8ee336a to 38f42b8 Compare January 31, 2023 18:10
@beeme1mr
Copy link
Member

Relates to: #41

@tcarrio
Copy link
Member

tcarrio commented Jan 31, 2023

Correct @beeme1mr . I am deprecating 7.4 support in this library shortly so this won't be a blocker soon.

See this comment for info: #41 (comment)

@kidager thank you for the PR! I will merge this after the 7.4 deprecation and coinciding changes to the SDK are merged.

@kidager
Copy link
Contributor Author

kidager commented Feb 1, 2023

I missed that PR (and thus comment).
Thank you for your responsiveness 🙏

@tcarrio
Copy link
Member

tcarrio commented Feb 10, 2023

I missed that PR (and thus comment). Thank you for your responsiveness pray

No worries! I've gotten the core SDK upgraded so that PHP 7.4 is deprecated. Next up, I'll be updated the contrib packages. For now, you should be able to rebase and make any necessary changes to support newer psr/log versions 🙂

@tcarrio
Copy link
Member

tcarrio commented Feb 12, 2023

This is going to be a bit difficult I would think- since the interfaces actually do change between these versions we may have to target the latest. Perhaps dropping only version 1 may work though

Signed-off-by: Jacem Chaieb <[email protected]>
@kidager
Copy link
Contributor Author

kidager commented Feb 15, 2023

This is going to be a bit difficult I would think- since the interfaces actually do change between these versions we may have to target the latest. Perhaps dropping only version 1 may work though

You're right, for now I've rebased and made the changes, the pre-commit checks are passing. Will push and check if the different builds are working fine, if that's the case then bingo.
If not, I'm afraid we'll have to target only the latest version.

@kidager kidager force-pushed the fix/psr-log-support branch from 38f42b8 to f17ec35 Compare February 15, 2023 11:13
composer.json Outdated Show resolved Hide resolved
@tcarrio
Copy link
Member

tcarrio commented May 16, 2023

@kidager this change is now applicable with the deprecation of PHP 7.4

I went ahead and bumped 1.1 out of the versions since it was incompatible, as we had discussed.

This unblocks utilizing OpenFeature with projects like Laravel. Thank you again for your contribution! 🎉

@tcarrio tcarrio force-pushed the fix/psr-log-support branch from 50573ec to 92786f6 Compare May 16, 2023 21:41
Signed-off-by: Tom Carrio <[email protected]>
@tcarrio tcarrio force-pushed the fix/psr-log-support branch from 92786f6 to c5916f0 Compare May 16, 2023 21:46
@tcarrio
Copy link
Member

tcarrio commented May 16, 2023

Well, sorry @kidager, there are more issues this caused than expected. We have the Flagd provider in open-feature/php-sdk-contrib and integration tests for the php-sdk against it in this repo- so we're seeing a bit of a chicken & egg scenario here.

If this takes much longer I'm going to branch off your changes so I'm not creating as much noise for you 🙏

@tcarrio
Copy link
Member

tcarrio commented May 16, 2023

And actually that was the last of the changes needed. LGTM 🚀

@tcarrio tcarrio merged commit f8a67c7 into open-feature:main May 16, 2023
tcarrio pushed a commit that referenced this pull request May 16, 2023
🤖 I have created a release *beep* *boop*
---


##
[2.0.1](2.0.0...2.0.1)
(2023-05-16)


### Miscellaneous Chores

* **deps:** update dependency php to v8.2.3
([#73](#73))
([27d1bbc](27d1bbc))
* **deps:** update dependency phpstan/phpstan to ~1.10.0
([#87](#87))
([f59da40](f59da40))
* **deps:** update dependency vimeo/psalm to ~5.8.0
([#72](#72))
([800119f](800119f))
* remove deprecated --no-suggest composer flag
([#89](#89))
([2adecfb](2adecfb))
* update psr/log
([#74](#74))
([f8a67c7](f8a67c7))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants