{"action":"created","issue":{"url":"https://api.github.com/repos/code-dot-org/code-dot-org/issues/18866","repository_url":"https://api.github.com/repos/code-dot-org/code-dot-org","labels_url":"https://api.github.com/repos/code-dot-org/code-dot-org/issues/18866/labels{/name}","comments_url":"https://api.github.com/repos/code-dot-org/code-dot-org/issues/18866/comments","events_url":"https://api.github.com/repos/code-dot-org/code-dot-org/issues/18866/events","html_url":"https://github.com/code-dot-org/code-dot-org/pull/18866","id":270819310,"number":18866,"title":"Refactor common auto-complete functions into helper","user":{"login":"joannepolsky","id":29499589,"avatar_url":"https://avatars3.githubusercontent.com/u/29499589?v=4","gravatar_id":"","url":"https://api.github.com/users/joannepolsky","html_url":"https://github.com/joannepolsky","followers_url":"https://api.github.com/users/joannepolsky/followers","following_url":"https://api.github.com/users/joannepolsky/following{/other_user}","gists_url":"https://api.github.com/users/joannepolsky/gists{/gist_id}","starred_url":"https://api.github.com/users/joannepolsky/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/joannepolsky/subscriptions","organizations_url":"https://api.github.com/users/joannepolsky/orgs","repos_url":"https://api.github.com/users/joannepolsky/repos","events_url":"https://api.github.com/users/joannepolsky/events{/privacy}","received_events_url":"https://api.github.com/users/joannepolsky/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-11-02T21:50:56Z","updated_at":"2017-11-02T22:07:59Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/code-dot-org/code-dot-org/pulls/18866","html_url":"https://github.com/code-dot-org/code-dot-org/pull/18866","diff_url":"https://github.com/code-dot-org/code-dot-org/pull/18866.diff","patch_url":"https://github.com/code-dot-org/code-dot-org/pull/18866.patch"},"body":"Wanted to refactor this before I submit the district auto-complete functionality. These are the shared constants and functions."},"comment":{"url":"https://api.github.com/repos/code-dot-org/code-dot-org/issues/comments/341572696","html_url":"https://github.com/code-dot-org/code-dot-org/pull/18866#issuecomment-341572696","issue_url":"https://api.github.com/repos/code-dot-org/code-dot-org/issues/18866","id":341572696,"user":{"login":"aoby","id":8405316,"avatar_url":"https://avatars0.githubusercontent.com/u/8405316?v=4","gravatar_id":"","url":"https://api.github.com/users/aoby","html_url":"https://github.com/aoby","followers_url":"https://api.github.com/users/aoby/followers","following_url":"https://api.github.com/users/aoby/following{/other_user}","gists_url":"https://api.github.com/users/aoby/gists{/gist_id}","starred_url":"https://api.github.com/users/aoby/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aoby/subscriptions","organizations_url":"https://api.github.com/users/aoby/orgs","repos_url":"https://api.github.com/users/aoby/repos","events_url":"https://api.github.com/users/aoby/events{/privacy}","received_events_url":"https://api.github.com/users/aoby/received_events","type":"User","site_admin":false},"created_at":"2017-11-02T22:07:59Z","updated_at":"2017-11-02T22:07:59Z","author_association":"CONTRIBUTOR","body":"Overall the functionality looks good, but I have some questions about the organization and design:\r\n\r\n 1. Is there a reason that SchoolAutoComplete is a class instead of a controller concern that can be applied to the controllers where it's needed? Alternatively it could be a module that is included (mixed in) if you wanted to keep it in lib and use it in other contexts than just controllers. In any case, it doesn't appear to maintain any state which is typically why I'd want a class. I do see that you are removing the singleton mixin, which is a good step (again since there's no state), but I think this would be cleaner & easier to use as a module that can be mixed in.\r\n\r\n 2. What is the purpose of the new `AutocompleteHelper` base class? Again I have the same questions as above. Does this need to be a class? It seems like we're creating extra hierarchy, and making this more cumbersome than it needs to be. Perhaps I just don't have enough context.\r\n\r\n 3. nit: \"Helper\" has a specific meaning in Rails (on that note, this could be a controller helper instead of concern or plain old module), and beyond that I tend to dislike that suffix as not really really adding any meaning. How is `AutocompleteHelper` more descriptive than `Autocomplete`, and then I find it odd that the derived (presumably more specific) class loses the suffix."}}
{"actor":{"display_login":"aoby"}}