You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@shindig.apache.org by "Jinhui Du (JIRA)" <ji...@apache.org> on 2009/06/25 03:41:07 UTC
[jira] Created: (SHINDIG-1099) Spec 0.9 album php shindig
implementation
Spec 0.9 album php shindig implementation
-----------------------------------------
Key: SHINDIG-1099
URL: https://issues.apache.org/jira/browse/SHINDIG-1099
Project: Shindig
Issue Type: New Feature
Components: PHP
Environment: php 5.2.9
Reporter: Jinhui Du
The code review url is http://codereview.appspot.com/87059
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-1099) Spec 0.9 album php shindig
implementation
Posted by "Jinhui Du (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12725484#action_12725484 ]
Jinhui Du commented on SHINDIG-1099:
------------------------------------
I just changed all the handlers to add the check logic. please review the CL http://codereview.appspot.com/89063
> Spec 0.9 album php shindig implementation
> -----------------------------------------
>
> Key: SHINDIG-1099
> URL: https://issues.apache.org/jira/browse/SHINDIG-1099
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Environment: php 5.2.9
> Reporter: Jinhui Du
> Assignee: Chris Chabot
> Fix For: trunk
>
>
> The code review url is http://codereview.appspot.com/87059
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (SHINDIG-1099) Spec 0.9 album php shindig
implementation
Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1099?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Chris Chabot resolved SHINDIG-1099.
-----------------------------------
Resolution: Fixed
Fix Version/s: trunk
Assignee: Chris Chabot
Patch looked good to me, and has been applied and committed, thanks!
The only thing I left out was this change (and related lines in the same file):
Index: php/src/social/service/MessagesHandler.php
+ try {
+ $service = Config::get('messages_service');
+ } catch (ConfigException $e) {
+ // Do nothing. If messages_service is not specified in the config file.
+ // All the requests to message handler will throw not implemented exception.
+ }
+ if ($service) {
+ $this->service = new $service();
+ }
In it's self it's a nifty trick that I think is a good idea, but I'm also a huge fan of consistency, inconsistent methodology means it will behave differently then people expect when messages is self not_implemented responding, but app data for instance isn't.. So my advice, implement the logic for all Handlers and submit that as a separate patch, and I'd be happy to commit that :)
> Spec 0.9 album php shindig implementation
> -----------------------------------------
>
> Key: SHINDIG-1099
> URL: https://issues.apache.org/jira/browse/SHINDIG-1099
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Environment: php 5.2.9
> Reporter: Jinhui Du
> Assignee: Chris Chabot
> Fix For: trunk
>
>
> The code review url is http://codereview.appspot.com/87059
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-1099) Spec 0.9 album php shindig
implementation
Posted by "Chris Chabot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-1099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723973#action_12723973 ]
Chris Chabot commented on SHINDIG-1099:
---------------------------------------
pps, it might be a good idea then if you do make a patch for that not implemented behavior to also check for empty config strings, could be a nice and simple way to indicate something is not supported by the container
> Spec 0.9 album php shindig implementation
> -----------------------------------------
>
> Key: SHINDIG-1099
> URL: https://issues.apache.org/jira/browse/SHINDIG-1099
> Project: Shindig
> Issue Type: New Feature
> Components: PHP
> Environment: php 5.2.9
> Reporter: Jinhui Du
> Assignee: Chris Chabot
> Fix For: trunk
>
>
> The code review url is http://codereview.appspot.com/87059
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.