Skip to content

Conversation

@ipundit
Copy link
Contributor

@ipundit ipundit commented Sep 5, 2023

Looks like a small regression failure introduced in the last merge to master. Fixed.

ipundit added 25 commits August 31, 2023 18:10
…at capitalization for attribute names are retained. So the short form for New York is now NY instead of ny. test/model/Author, Book, and Venue now have those strtoupper() workarounds in getter/setter methods removed as it's done in Model.php
…rcased in Model.php in order to be case insensitive.
…at capitalization for attribute names are retained. So the short form for New York is now NY instead of ny. test/model/Author, Book, and Venue now have those strtoupper() workarounds in getter/setter methods removed as it's done in Model.php
…rcased in Model.php in order to be case insensitive.
…cord

# Conflicts:
#	test/ActiveRecordTest.php
#	test/models/Book.php
…Disable it so that regression does not timeout.
…d#35:

Authors table now has a column that is camel cased: firstName
Added tests that demonstrate case insensitivity when accessing this firstName column
- Correct link to contributing.md
…cord

# Conflicts:
#	composer.json
#	scripts/TestCommand.php
@shmax
Copy link
Contributor

shmax commented Sep 5, 2023

Ah, crap, I didn't notice the warnings in 8.2. I guess we'll have to see if we can figure out how to promote warnings to errors so this doesn't keep happening...

@shmax shmax merged commit 8995b42 into php-activerecord:master Sep 5, 2023
@ipundit
Copy link
Contributor Author

ipundit commented Sep 5, 2023

I set this flag in my php.ini file:

error_reporting = E_ALL & ~E_NOTICE

perhaps

error_reporting = E_ALL

makes more sense if we want a really clean test output

@shmax
Copy link
Contributor

shmax commented Sep 5, 2023

Well, in this case the problem isn't that warnings aren't being raised, but that the tests pass even when they do.

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.

2 participants