laminas / laminas-db

Database abstraction layer, SQL abstraction, result set abstraction, and RowDataGateway and TableDataGateway implementations
https://docs.laminas.dev/laminas-db/
BSD 3-Clause "New" or "Revised" License
119 stars 66 forks source link

Revert #240 - reverted bump of `laminas/laminas-coding-standard` to `^2.3.0` #242

Closed Ocramius closed 2 years ago

Ocramius commented 2 years ago

Reverts laminas/laminas-db#240 Fixes #241

/cc @ghostwriter @arueckauer

Ocramius commented 2 years ago

7.3 + latest still fails, but better than a completely broken system:

FAILURES!
Tests: 1230, Assertions: 2077, Failures: 1, Warnings: 8, Skipped: 41, Incomplete: 80, Risky: 58.
arueckauer commented 2 years ago

Did not expect it to fail that bad. Thanks for both of your efforts!

Ocramius commented 2 years ago

@arueckauer heh, green CI on this was very unexpected anyway :D

ghostwriter commented 2 years ago

Why revert, instead of fixing the Types of the 2nd and 3rd arguments of var_export and str_replace respectively?

When a “feature” is released, and caused an undesired outcome in production; how do you fix the issue?

fix the issue in a minor patch

Create a separate bugfix branch with the fix

revert the feature

What do you do here?

Re-open the feature PR, fix the issue, re-merge?

Create a new feature Pr with code from feature branch with the additional bugfix commits?

I want to learn and understand the thought process behind the decisions so I can apply them correctly.

Thank you for you time.

Ocramius commented 2 years ago

The question is also timing: I didn't have time to investigate nor fix this, so a full revert was quick and viable.

Re-applying the patch is possible afterwards, in 2.15.x or later, but for now, allowing usage of 2.14.1 takes priority.

dstockto commented 2 years ago

We ran into an issue w/ 2.14.0 as well - using non-strings (like ints) as literal column values mapped to a column name also caused a type error.