Skip to content
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

Simple way to make working with Paris more beautiful? #62

Closed
wants to merge 6 commits into from

Conversation

Lapayo
Copy link
Contributor

@Lapayo Lapayo commented May 16, 2013

When I started working with Paris, I wondered, why there is no function like that and I added my own. Is there a reason why this functionality wasnt already in Paris? :D

Commit Text:
Makes working with models more beautiful (in my opinion). :-)

Examples:
User::find_many();
User::where('name', 'lapayo')->find_one();

Makes working with models more beautiful (in my opinion). :-)


Examples:
User::find_many();
User::where('name', 'lapayo')->find_one();
@treffynnon
Copy link
Collaborator

Thank you for the pull request. One reason I can see for this not already existing is that it does not work in PHP 5.2 due to the use of the static keyword: https://travis-ci.org/j4mie/paris/jobs/7214392

@Lapayo
Copy link
Contributor Author

Lapayo commented May 16, 2013

Hmm you are right, haven´t thought of that, because I always use the newest versions. I think even the PHP devs dropped support for 5.2 (http://php.net/releases/5_2_16.php)

@michaelward82
Copy link
Contributor

Would it be acceptable to code this as some extra goodness for PHP 5.3+ users? Make it so it fails gracefully in 5.2

@treffynnon
Copy link
Collaborator

@Lapayo That is correct they have dropped official support for 5.2 (and quite some time ago at that), but please see my comments here for the arguments: j4mie/idiorm#77 (comment)

@treffynnon
Copy link
Collaborator

@michaelward82 how would you propose it working without breaking PHP 5.2 support? I haven't given it any thought so any suggestions welcome.

@Lapayo
Copy link
Contributor Author

Lapayo commented May 16, 2013

@treffynnon Simply using self instead of static should not break 5.2 support as long as they wont call the magic function.

@Lapayo
Copy link
Contributor Author

Lapayo commented May 16, 2013

@michaelward82 Although I somehow don´t see your response here on github: Yes, your solution would be the better one, because it would not break 5.2, even when calling the magic method.
Should I make a new Pull-Request?

@treffynnon
Copy link
Collaborator

Yeah that would be great. Also please include some tests and some documentation.

@michaelward82
Copy link
Contributor

Not sure what happened to my comment lol. (I suggested feature detection whilst avoiding the use of static)

@Lapayo
Copy link
Contributor Author

Lapayo commented May 16, 2013

At first: Sorry for all the commits :D Didn´t knew they all get posted here.

I mentioned it in the documentation, the readme, created a simple test, and fixed it for 5.2 users. :-)

I hope this is enough, otherwise I will extend it this evening ;-)

@ghost ghost assigned treffynnon May 23, 2013
@treffynnon
Copy link
Collaborator

I am very busy with some client work at the moment, but I think this is at a point where it can be merged and prepared for the next release. I am currently expecting to make the next release beginning to mid June.

@treffynnon
Copy link
Collaborator

Merged in commit 433c2f5

@treffynnon treffynnon closed this Aug 28, 2013
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.

None yet

3 participants