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

refactor managers and handlers. #1426

Merged
merged 13 commits into from
Jun 1, 2016
Merged

refactor managers and handlers. #1426

merged 13 commits into from
Jun 1, 2016

Conversation

stopfstedt
Copy link
Member

shedding some weight for the summer. 👙

@homu
Copy link
Contributor

homu commented May 21, 2016

☔ The latest upstream changes (presumably #1399) made this pull request unmergeable. Please resolve the merge conflicts.

@stopfstedt
Copy link
Member Author

recap:

  • standardized CRUD-method names in managers, then consolidated them as much as possible into BaseManager and DTOManager.
  • refactored handlers to use managers rather than extending them. then consolidated all handers into one class BaseHandler and rewired everything.
  • updated all classes that are interfacing with handlers and managers, most notably controllers, voters and commands.
  • minor cleanup like code comments corrections and method/class-name case corrections.

@stopfstedt
Copy link
Member Author

140 files less.

~/dev/projects/ilios on master
$ find src -type f | wc -l
991

vs.

~/dev/projects/ilios on refactor_handlers
$ find src -type f | wc -l
848

@jrjohnson
Copy link
Member

Holding off on merging until post 3.11

@jrjohnson
Copy link
Member

Reviewed as much as possible. Great stuff, lots of less code 💯 this is ready to go once 3.12 is released because I would like to merge it with plenty of time to test.

@jrjohnson jrjohnson merged commit ea83c60 into ilios:master Jun 1, 2016
@stopfstedt stopfstedt deleted the refactor_handlers branch June 17, 2016 17:20
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.

None yet

4 participants