Skip to content

Widen dependency versions#64

Open
pkruithof wants to merge 1 commit into
Eppo-exp:mainfrom
pkruithof:widen-dependencies
Open

Widen dependency versions#64
pkruithof wants to merge 1 commit into
Eppo-exp:mainfrom
pkruithof:widen-dependencies

Conversation

@pkruithof

Copy link
Copy Markdown

🎟️ Fixes issue
📜 Design Doc: link if applicable

Motivation and Context

We're using Eppo with this library and want to upgrade to Symfony 8. Currently the cache dependency is preventing that.

Description

I've widened the requirement for this dependency, and fixed some low-hanging fruit I encountered while doing so:

  • remove composer.lock as it's not recommended for libraries
  • expanded the test suite so it tests this library with both the lowest and highest versions of the dependencies

How has this been documented?

How has this been tested?

  • running make test

runs-on: ubuntu-latest
strategy:
matrix:
deps:

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ideally you'd want to add different PHP versions to this as well. I'll leave that out of scope for this PR though.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not sure why there were no workflows triggered for this PR though, the file syntax seems ok and I don't see any errors in the actions tab.

use Psr\Http\Client\ClientExceptionInterface;
use Psr\Http\Client\ClientInterface;
use Psr\Http\Message\RequestFactoryInterface;
use Teapot\StatusCode\RFC\RFC7231;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Having a dependency just for readable http status codes, and enforcing that on all who install this library seems a bit wasteful. But if you insist on keeping this I can revert this, although in that case I would suggest using symfony/http-foundation as it's a far more common dependency.

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.

1 participant