search

slimphp / Slim-Psr7

PSR-7 implementation for use with Slim 4
MIT License
133 stars 45 forks source link

Some minor improvements #274

Closed williamdes closed 1 year ago

williamdes commented 1 year ago

About 966d0fffd845a4e63a69fb157ed4e77b0ab1041b it is similar to https://github.com/phpmyadmin/sql-parser/commit/a20131487496892d4111c1cbe0025fee35180cce In Debian we need the tests folder to run auto-package tests on packages. Example: https://salsa.debian.org/php-team/pear/php-zumba-json-serializer/-/jobs/3521669#L312 or https://ci.debian.net/data/autopkgtest/unstable/amd64/p/phpmyadmin-sql-parser/27794800/log.gz

williamdes commented 1 year ago

Hi @l0gicgate

Can you review this in priority before the next release please ? It is really important for me to be able to import your library into Debian: https://salsa.debian.org/php-team/pear/php-slim-psr7

And be able to package it. I can package it without tests, but you know it's not safe for end users, etc..

coveralls commented 1 year ago

Coverage Status

Coverage: 100.0%. Remained the same when pulling f69001503b7d0e9ae57ba0d876ca78a7eb30c4c2 on williamdes:improvements into a5327c8784b085fc33915394ca759f8b13ee4354 on slimphp:master.

l0gicgate commented 1 year ago

@williamdes it appears that php nightly is not supported by our downstream dependencies at the moment. I don't mind having CI run against nightly but it makes it look like the PR is failing when it should not.

@akrabat any thoughts on this? Should we run against nightly on our other repos too or only add when we know our downstream deps have caught up?

akrabat commented 1 year ago

The PHP nightly test should be configured in our workflow to be allowed to fail really.

akrabat commented 1 year ago

The other changes in the PR look good to me.

l0gicgate commented 1 year ago

@akrabat unfortunately github actions don’t support us to allow failure for experimental matrixes 😕

There’s a long-standing issue about this that I’ve been tracking since 2020: https://github.com/actions/runner/issues/2347

I don’t mind adding it, it’ll look like every PR is failing though.

akrabat commented 1 year ago

Ah, right. Yes, this is fine as we can set the branch rules to allow merge regardless of whether nightly passes or not.

akrabat commented 1 year ago

Thanks @williamdes. Sorry that it took so long to get merged.

williamdes commented 1 year ago

thank you for merging this, it will help for the Debian packaging It joined Debian with success: https://tracker.debian.org/pkg/php-slim-psr7 🎉