Include the encoding parameter in the constructor

[chrismld]

The encoding parameter (optional) it's included in the constructor but it was not included in this static class. So now, when you use the RollingFile extension method you'll have the option to send the encoding parameter. Not quite sure yet how to send this parameter from the configuration file.

I took this from the issue #337

[nblumhardt]

Thanks for the PR!

Just linking the matching RollingFile() change so that we can discuss the two here:

serilog/serilog-sinks-rollingfile#10

Purely out of curiosity, which encoding do you need to write log files in, and why is that? I'm finding it's unusual to see output other than UTF-8 these days :-)

Cheers,
Nick

[chrismld]

got it @nblumhardt

In my case UTF-8 works but without the UTF8 Identifier (which in other words is the BOM character), so I needed to use the constructor like this: new UTF8Encoding(false). And to give more background on this, I'm sending the logs to AWS Firehose which then sends the data to Redshift and because the file (more specific, the first line) had the BOM character, I couldn't upload the data to Redshift.

I would like to have this configuration in file, but couldn't figure out how to do that :(

[nblumhardt]

I thought that might be the case, thanks for the follow-up. Would you consider #9 viable? I don't personally believe we should be emitting the BOM by default. If so, help getting #9 in would be awesome.

[chrismld]

@nblumhardt sounds reasonable to me :) ... I can work on that but what about this pull request, don't you think it's needed? if not, then I'll create a separate pull request for #9

[nblumhardt]

Great, thanks for the follow-up.

I would rather avoid adding more to the File(...) argument list if we can avoid it, at least in the short term. If we get more feedback that it's a common scenario we could consider it, but:

WriteTo.Sink(new FileSink(..., Encoding.X, ...)

...is a pretty reasonable workaround for the edge cases and is supported today.

Cheers!

[chrismld]

Got it, no problem and you're right, that workaround makes sense too :) I'll close this PR and create another one