Skip to content
This repository was archived by the owner on Mar 24, 2026. It is now read-only.

Fixed errors in cake/Fortune, cake/db, php-orm/db#682

Merged
hamiltont merged 1 commit intoTechEmpower:masterfrom
remiq:cake-fortune-fix
Aug 7, 2014
Merged

Fixed errors in cake/Fortune, cake/db, php-orm/db#682
hamiltont merged 1 commit intoTechEmpower:masterfrom
remiq:cake-fortune-fix

Conversation

@remiq
Copy link
Copy Markdown
Contributor

@remiq remiq commented Dec 29, 2013

Fixed syntax error in cake/app/Controllers/FortunesController
Corrected name of table in cake/app/Model/World.php.
Corrected name of table in php/models/World.php.

Also:
Changed method of loading fortunes from raw SQL to production-grade ORM.

Changed method of loading fortunes from raw SQL to  production-grade ORM.
Corrected name of table in cake/app/Model/World.php.
Corrected name of table in php/models/World.php.
@msmith-techempower
Copy link
Copy Markdown
Member

@remiq While the tests run, the output do not match the rules laid out here: http://www.techempower.com/benchmarks/#section=code

Here is your output: https://gist.github.com/msmith-techempower/8404715

Specifically, the formatting for db and query are too verbose. The db test should always return a single JSON object with the keys 'id' and 'randomNumber' while the query test should always return a JSON array containing 1-500 objects with the keys 'id' and 'randomNumber'.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this syntax is ok in php 5.4 which should be used in tests.

@jlucier-techempower
Copy link
Copy Markdown
Contributor

We ran your code and only json and plaintext passed.

Output: https://gist.github.com/jlucier-techempower/ae970903aedb72eeeeda

db and query failed validation. The rules for those tests can be found here: http://www.techempower.com/benchmarks/#section=code&hw=peak&test=json

@hamiltont
Copy link
Copy Markdown
Contributor

Re:

Changed method of loading fortunes from raw SQL to production-grade ORM

I think you should also be updating the benchmark_profile

@hamiltont
Copy link
Copy Markdown
Contributor

Oh, scratch that. cake already lists "orm": "Full", so this really is a bug fix. Nice!

@hamiltont hamiltont merged commit f3d407d into TechEmpower:master Aug 7, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants