#505, Allow custom Flapjack branding.

This is to deal with #505, Allow custom Flapjack branding.

The change allows for an image_logo_path setting in the config that points to a custom logo file. 

I'm not 100% sure if this PR should be considered. I'm unhappy with it for two reasons:

1. The logo is copied in from the path into a /public/img/custom folder (created when the Sintra app starts up). This feels like a hack. If I'd started this again I would have used sendfile from the image's default location, though this might have caused performance / security issues. 

2. I had to comment out a test that works in isolation (defaulting to the standard logo if nothing configured). The test will sometimes fail, possibly if the primary spec for the logo executes first and the @config variable gets set. I can't bend my head around this - the start method should load the config, and it's being called independently in each of the tests. There's some weird interdependence between specs in the web_spec.rb file, but I can't for the life of me figure it out - at least not in the time available.

Any feedback appreciated. 

Agree. I'd want to keep it using the same idioms you've already laid out,
so yes. As an aside, how do you feel about the use of 'unless' in
preference to 'if !'? Do you find that more readable?


On Tue, Jun 24, 2014 at 11:57 AM, Ali Graham <notifications@github.com>
wrote:

> In lib/flapjack/gateways/web.rb:
>
> > @@ -76,6 +76,23 @@ def start
> >              @base_url = dummy_url
> >            end
> >
> > +          logo_image_path = @config['logo_image_path']
> > +          if logo_image_path
> > +            if !File.file?(logo_image_path)
>
> Rather than start with if ! it might be good to invert the clauses -- a
> matter of taste though.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/flapjack/flapjack/pull/544/files#r14110361>.
>