-
Notifications
You must be signed in to change notification settings - Fork 125
Feature/initial attempt #72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
{ | ||
gzipStream.Write(buffer, 0, bytesRead); | ||
} | ||
inputStream.CopyTo(gzipStream, bufferSize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such a small buffer size is going to result in sub-par compression ratios - what's the purpose of overriding the default buffer size here? (I guess the default is 64K?)
var buffer = new byte[1024]; | ||
int bytesRead; | ||
int bufferSize = 1024; | ||
var buffer = new byte[bufferSize]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think buffer
is being used?
@@ -174,7 +190,8 @@ void AlignCurrentFileTo(DateTime now, bool nextSequence = false) | |||
{ | |||
var directoryFileName = Path.GetFileName(directoryFile); | |||
|
|||
if (!(directoryFileName.Contains("-GZip"))) | |||
// Not sure if we can assume log files will always be .txt? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope 😄
The filename (and extension) are configurable by the user
/// </summary> | ||
/// <param name="logFile"> The log file to be compressed </param> | ||
/// <param name="logDirectory"> The directory that holds logFile </param> | ||
public void GZipCompress(string logFile, string logDirectory) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why public
?
@AdamUdovich I think #80 is going to enable this; I'm catching up and doing a bit of housekeeping, so I'll close the PR here - kudos for pushing it forwards :-) |
Diff for rolling compression.