Code review comment for lp:~mnordhoff/loggerhead/config_app_stuff

Revision history for this message
Matt Nordhoff (mnordhoff) wrote :

Good morning,

Some improvements to loggerhead.apps.config.Project's interaction with BranchWSGIApp:

* Use BranchWSGIApp.served_url instead of inventing a 'branch_url' attribute.

* Don't set so many attributes on the BranchWSGIApp object, just 'description' and 'name'. (The constructor can handle the rest.)

* Remove '_src_folder' and '_view_config' from view_data_by_name as they were unused.

* Fix a latent semi-bug: it called BranchWSGIApp(friendly_name=view_name), then set BranchWSGIApp.friendly_name to friendly_name. (I decided to go with friendly_name.)

This was inspired by my investigation into bug #375948 (the branch_url method got clobbered), although I already fixed it a different way (just by renaming it).

Mostly this isn't very important, but I feel that reducing the number of attributes being set is safer, plus I did a little cleanup.

You should sanity-check my decision regarding served_url. This means it'll fall back to public_branch if it's set, but that seems okay to me. Aside from that, I think it's fine. Probably. ;D

So...yeah. :)

« Back to merge proposal