Code review comment for lp:~bloodearnest/canonical-identity-provider/talisker

> On Mon, Jul 11, 2016 at 3:57 PM, Ricardo Kirkner
> <email address hidden> wrote:
> > +1 to logging to console (one of the 12-factor app tenants) as it makes the
> application more portable (environment independence)
> >
> > Diff comments:
> >
> >>
> >> === modified file 'requirements.txt'
> >> --- requirements.txt 2016-07-03 22:36:48 +0000
> >> +++ requirements.txt 2016-07-06 16:05:45 +0000
> >> @@ -34,7 +36,6 @@
> >> raven==5.6.0
> >> requests-oauthlib==0.4.2
> >> six==1.10.0
> >> -statsd==3.1
> >
> > why is this removed?
>
> Because talsisker brings it in as a dep (and upgrades it in the process)
>
> The idea was to provide one place for our core cross-app deps to be
> versioned/pulled from. Makes it easier to ensure we are on the same
> versions across apps, and reduces the number of deps the app itself
> has to version/manage.
>
> At the moment, that's
>
> gunicorn
> statsd
> requests
>
> Make sense?

At some level, yes, but then it seems odd to rely on the implicit dependency when the code depends on the library explicitly. Also, it means that if for any reason we need to bump these lib's versions we will have to bump talisker first (for no real reason).

>
> >> testresources==0.2.7
> >> timeline==0.0.4
> >> timeline-django==0.0.4
> >
> >
> > --
> > https://code.launchpad.net/~bloodearnest/canonical-identity-
> provider/talisker/+merge/297455
> > You are the owner of lp:~bloodearnest/canonical-identity-provider/talisker.