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

Fixing issue #87 #90

Merged
merged 4 commits into from
Nov 21, 2013
Merged

Fixing issue #87 #90

merged 4 commits into from
Nov 21, 2013

Conversation

hpatoio
Copy link
Contributor

@hpatoio hpatoio commented Nov 18, 2013

I left a couple of comments in the code. Feel free to change what you want. Hope you like it.

@@ -44,6 +44,12 @@ class PageController extends ContainerAware
*/
public function homeAction($repository = 'doctrine/orm')
{
return array('repository' => $repository);

$redis_reader = $this->container->get('stats_reader');
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix CS :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the problem exactly here ? I'm not good at CS :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

camelCase :)

Copy link
Contributor

Choose a reason for hiding this comment

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

@liuggio
Copy link
Member

liuggio commented Nov 18, 2013

great job 👍
about the ux?
Is it still WIP?

@hpatoio
Copy link
Contributor Author

hpatoio commented Nov 18, 2013

What you mean ? At the moment the counter appears at the top left, I like it this way but if you have advice just tell me.

@liuggio
Copy link
Member

liuggio commented Nov 21, 2013

@hpatoio for me is great, could you fix the CS fix?

@hpatoio
Copy link
Contributor Author

hpatoio commented Nov 21, 2013

Sure, I can run https://github.com/fabpot/PHP-CS-Fixer on all files if you want.

@liuggio
Copy link
Member

liuggio commented Nov 21, 2013

maybe we could close this PR with that simple CS fix,
and then if is needed, you could do another PR with CS fixer

What do you think?

@hpatoio
Copy link
Contributor Author

hpatoio commented Nov 21, 2013

OK. Give me few minutes and I'll fix what @stewe pointed out.

liuggio added a commit that referenced this pull request Nov 21, 2013
@liuggio liuggio merged commit c47bb43 into PUGX:master Nov 21, 2013
@liuggio
Copy link
Member

liuggio commented Nov 21, 2013

is online :)

@hpatoio
Copy link
Contributor Author

hpatoio commented Nov 21, 2013

Good. BTW. A number format for the counter would have been a good idea. Next release.

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.

3 participants