These are chat archives for eventum/eventum

19th
Nov 2015
Bryan Alsdorf
@balsdorf
Nov 19 2015 02:58
Hmmmm. I'd say go ahead and make a PR for the first one and if all looks good and works well then I think it will be fine to convert a pages incrementally and push them to master
Elan Ruusamäe
@glensc
Nov 19 2015 07:27
created, i'll let you know when it's ready to merge
i'll take view.php as next, i hope you don't need to modify it soon :)
Elan Ruusamäe
@glensc
Nov 19 2015 19:47
@balsdorf Misc::displayErrorMessage($msg) renders $msg with error template and exits. i think some exception would be appropriate in controllers, what you think?
altho i could add $this->error() method for now
Elan Ruusamäe
@glensc
Nov 19 2015 20:52
@balsdorf view.php ported, but could you check maybe the project id check can be dropped as issue::access already does the same thing?
and do you look at individual commits if you review my pulls?
Bryan Alsdorf
@balsdorf
Nov 19 2015 21:43
@glensc Changing to an exception would probably be the "proper" way to do it. I wouldn't want to change it to $this->error incase a workflow or other non controller code needs to call it
I generally just look at files changed, not individual commits
Bryan Alsdorf
@balsdorf
Nov 19 2015 21:57
The explicit project check (on line 117) can be dropped since as you pointed out issue::access checks that
Everything looks good to me, I haven't run it yet just read the diffs. I barely recognize the code :)
You can merge it or if you like I'll check it into my machine tomorrow and run it
Elan Ruusamäe
@glensc
Nov 19 2015 22:07
I think i port more classes still
Ordering by file size :)
And maybe checking most accessed files from my system :)
Bryan Alsdorf
@balsdorf
Nov 19 2015 22:09
When you do the update page you can probably reuse the logic in getColumnsForDisplay. Not sure best way to share logic like that
Elan Ruusamäe
@glensc
Nov 19 2015 22:11
If multiple controllers share method, there should be new class for that. Afaik zend calls them view helpers
Maybe should use zend mvc here?
It's all new to me
I think the harder part is moving foo.php to these classes, later can add real framework