-
Notifications
You must be signed in to change notification settings - Fork 14
Fix documentation that led to Issue #37 #42
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
Conversation
…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.
# Conflicts: # lib/Model.php
…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
…vity() is enough.
- Added test case to demonstrate how to use a select of specific columns on equality to a primary key
- Added test case to demonstrate that find("conditions") will always return an array, even if ['limit'=>1]
Well, we're gearing up to do the first major version release in more than a decade, here, so if you want to break stuff now is the time to do it. For what it's worth there are a few other API-breaking changes that I felt were necessary for the benefit of static checking (namely, the various As for whether "conditions" should be a singular model or array of them, well, I have no idea. I had to analyze all this in some depth when I was writing the PHPStan extension for My first thought was "ah, it's simple! If there's only one argument, then you only get one record back!". But that gets violated by this: $books = Book::find([1, 3, 8]); // arrayso I thought "oh, okay, well if there's only one argument but it's an array, then we go with array". But that gets contradicted by this: $book = Book::find(['name'=>'Ubik']); // singleAnd so on. You clearly think that I think convention is going to have to drive all this, but I don't really know where to go for the source of truth. This library was launched over a decade ago and was modeled after Ruby's implementation of Active Record, so if you really want to help sort it all out you might see if you can make sense of their documentation: https://guides.rubyonrails.org/active_record_basics.html I actually find the docs kind of scattered and hard to read, so even better might be to get Ruby working locally and maybe play around with it. edit: oh, and I forgot to mention; I wasn't able to find a "conditions"-style interface in the Ruby docs, so maybe it's not a thing, anymore? |
…st files to conform to project standard.
|
Nice, thank you! |
shmax clarified the expected return values of Model::find() dependent on its input. See #37. This is confusing behavior for a function, but to change it would lead to breaking changes on users of this base library.
This PR does the next best thing, which is to clarify the documentation in the function header comments and also in the test cases.
Also did some cleanup of the
.gitignorefile. Usually package-lock.json would be source controlled so that you know which versions of dependencies work and when you'd have to do acomposer updateafter a pull, but since composer.lock was ignored, I've also ignored package-lock.json.