Skip to content
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

Refactor Log method #1684

Closed
wants to merge 1 commit into from
Closed

Refactor Log method #1684

wants to merge 1 commit into from

Conversation

yahiheb
Copy link
Collaborator

@yahiheb yahiheb commented Jul 1, 2019

No description provided.

@lontivero
Copy link
Collaborator

The code is okay but I doesn't add much value IMO. Maybe, we could take the opportunity and remove all the encryption parts that we are not using. @nopara73?

@nopara73
Copy link
Contributor

nopara73 commented Jul 1, 2019

Didn't look at the code yet, but agree with @lontivero about removing the encryption part.

}

if (level < MinimumLevel)
if (Modes.Count == 0 || !IsOn() || level < MinimumLevel)
{
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes no sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It simply groups the two if statements.

else
{
File.AppendAllText(FilePath, finalFileMessage);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does, but since we're removing encryption this won't either.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about the encryption part. I simply changed the code to be easily readable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants