Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Fix comments, typos and add property validation to Microsoft.Extensions.Logging.AzureAppServices #509

Merged
merged 1 commit into from
Nov 4, 2016

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Nov 3, 2016

Fixes: #508 #506 #507

@pakrym pakrym changed the title Fix comments and typos and add property validation to Microsoft.Extensions.Logging.AzureAppServices Fix comments, typos and add property validation to Microsoft.Extensions.Logging.AzureAppServices Nov 3, 2016
get { return _blobName; }
set
{
if (value == null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is empty string fine?

@pakrym pakrym force-pushed the pakrym/fix-comments branch from b33b434 to 50a3881 Compare November 3, 2016 21:27
@pakrym
Copy link
Contributor Author

pakrym commented Nov 3, 2016

/cc @BrennanConroy @pranavkm


/// <summary>
/// Gets or sets a time to wait between checking for blob log batches.
/// Gets or sets a time to wait between checking for blob log batches. Defaults to 5 seconds.
Copy link
Member

Choose a reason for hiding this comment

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

Newline and <c></c>?

/// <summary>
/// Gets or sets a strictly positive value representing the maximum log size in bytes. Once the log is full, no more message will be appended.
/// Gets or sets a strictly positive value representing the maximum log size in bytes.
/// Once the log is full, no more message will be appended.
Copy link
Member

Choose a reason for hiding this comment

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

messages

@BrennanConroy
Copy link
Member

🚢 🇮🇹

@pakrym pakrym force-pushed the pakrym/fix-comments branch from 50a3881 to bde89a0 Compare November 3, 2016 22:12
@pakrym pakrym merged commit 1c9dcb0 into dev Nov 4, 2016
Copy link
Member

@Eilon Eilon left a comment

Choose a reason for hiding this comment

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

@pakrym please address these comments too.

get { return _outputTemplate; }
set
{
if (string.IsNullOrWhiteSpace(value))
Copy link
Member

Choose a reason for hiding this comment

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

Argh, why are we checking for whitespace??

get { return _blobName; }
set
{
if (string.IsNullOrWhiteSpace(value))
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

{
if (value < 0)
{
throw new ArgumentOutOfRangeException(nameof(value), $"{nameof(BackgroundQueueSize)} must be positive or 0.");
Copy link
Member

Choose a reason for hiding this comment

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

... must be non-negative.

@pakrym pakrym deleted the pakrym/fix-comments branch November 4, 2016 21:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants