Hello!

On Mon, Aug 14, 2017 at 06:00:53PM +0000, Eran Kornblau wrote:

> >
> > -----Original Message-----
> > From: nginx-devel [mailto:nginx-devel-bounces@nginx.org] On Behalf Of Maxim Dounin
> > Sent: Monday, August 14, 2017 8:34 PM
> > To: nginx-devel@nginx.org
> > Subject: Re: Add support for buffering is scripted logs
> >
> > > Ok, so is that a final 'no' for this whole feature, or is there is anything else I can do to get this feature in?
> >
> > It is certainly not a "final no". As I wrote in the very first comment, a) it's just a quick note, nothing more, and b) the feature is questionable. If a good implementation will be submitted, we can consider committing it.
> >
> That's good, I thought you were just rejecting politely :)

Well, I don't think there is a big difference, actually.

> It would be really great if you could point me to specific parts you think look bad.
> For example, I'm guessing that you don't like the callbacks I added to open file cache,
> but I was thinking that it's better to do it this way than to duplicate large chunks of code
> and write an open file cache specific to log, please let me know if you think otherwise.
> Any feedback you can provide will be appreciated

In no particular order:

- there are various style issues;

- you've dropped "simulate successful logging" comments which
where present for a reason;

- introducing separate ngx_open_file_cache_ext_t structure looks
completely pointless;

- it looks there are too many callbacks, it should be possible to
omit at least some of them.

Sorry, but please don't expect any further review / comments
though.

--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel