GitHub PullRequestReviewCommentEvent at 2014-04-17 07:56:59
Repository: MagentoConnectorBundle
Actor: gquemener
Payload: {"comment":{"url":"https://api.github.com/repos/akeneo/MagentoConnectorBundle/pulls/comments/11722395","id":11722395,"diff_hunk":"@@ -41,14 +46,16 @@ class HasValidCredentialsValidator extends ConstraintValidator\n \n     /**\n      * @param WebserviceGuesser   $webserviceGuesser\n-     * @param MagentoUrlValidator $magentoUrlValidator\n+     * @param HasValidSoapUrlValidator $hasValidSoapUrlValidator\n      */\n     public function __construct(\n         WebserviceGuesser $webserviceGuesser,\n-        MagentoUrlValidator $magentoUrlValidator\n+        HasValidSoapUrlValidator $hasValidSoapUrlValidator,\n+        HasValidApiUrlValidator $hasValidApiUrlValidator\n     ) {","path":"Validator/Constraints/HasValidCredentialsValidator.php","position":29,"original_position":29,"commit_id":"96aa6f81ac21dba9b22a5157e83c3b67f0a8d331","original_commit_id":"96aa6f81ac21dba9b22a5157e83c3b67f0a8d331","user":{"login":"gquemener","id":668604,"avatar_url":"https://avatars.githubusercontent.com/u/668604?","gravatar_id":"e09dec30b9ce941666a1ba3544213791","url":"https://api.github.com/users/gquemener","html_url":"https://github.com/gquemener","followers_url":"https://api.github.com/users/gquemener/followers","following_url":"https://api.github.com/users/gquemener/following{/other_user}","gists_url":"https://api.github.com/users/gquemener/gists{/gist_id}","starred_url":"https://api.github.com/users/gquemener/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gquemener/subscriptions","organizations_url":"https://api.github.com/users/gquemener/orgs","repos_url":"https://api.github.com/users/gquemener/repos","events_url":"https://api.github.com/users/gquemener/events{/privacy}","received_events_url":"https://api.github.com/users/gquemener/received_events","type":"User","site_admin":false},"body":"Injecting validator inside other validator looks kind of messy.\r\n\r\nValidator should validate one aspect of a value and should not depend on an other aspect.\r\nPlus, what happens if you define both constraint HasValidSoapUrlValidator and HasValidCredentialsValidator on an object?\r\nConstraint will be runned twice, too bad...","created_at":"2014-04-17T07:56:59Z","updated_at":"2014-04-17T07:56:59Z","html_url":"https://github.com/akeneo/MagentoConnectorBundle/pull/103#discussion_r11722395","pull_request_url":"https://api.github.com/repos/akeneo/MagentoConnectorBundle/pulls/103","_links":{"self":{"href":"https://api.github.com/repos/akeneo/MagentoConnectorBundle/pulls/comments/11722395"},"html":{"href":"https://github.com/akeneo/MagentoConnectorBundle/pull/103#discussion_r11722395"},"pull_request":{"href":"https://api.github.com/repos/akeneo/MagentoConnectorBundle/pulls/103"}}}}