Skip to content
This repository has been archived by the owner on May 1, 2019. It is now read-only.

Fix: removed unnecessary api call #273

Merged

Conversation

ins0
Copy link
Contributor

@ins0 ins0 commented Dec 12, 2014

Fixes unnecessary API call to Github

@localheinz
Copy link
Member

@ins0

I'd prefer to see ZfModule/Controller/IndexController.php#L66 removed - because why make a call to an API if there are chances to return before the expression result can be used, such as ZfModule/src/ZfModule/Controller/IndexController.php#L69-L71?

What do you think?

Good catch, though!

@localheinz localheinz self-assigned this Dec 12, 2014
@ins0
Copy link
Contributor Author

ins0 commented Dec 12, 2014

that also was also my first thought - but the api call on L66 is currently necessary because of the current cache logic $response->getStatusCode() == 304 - otherwise we don't know, if the cache should be used or not. or have i missed something?

but i agree, the current cache implementation works - but looks awful.
i would prefer to see a smart reference there and a non persistent cache that gets tossed after x time. so we are not forced to call the api every time we want to access a repository.

@localheinz
Copy link
Member

@ins0

👍

localheinz added a commit that referenced this pull request Dec 12, 2014
@localheinz localheinz merged commit 48372fc into zendframework:master Dec 12, 2014
@ins0 ins0 deleted the fix/unnecessary-double-api-call branch December 16, 2014 12:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants