toast notifications(+) #17

Nitpick: newline

Nitpick: One liner? if (this.shownStopwatch is null) return;

Nitpick: One liner? `if (this.shownStopwatch is null) return;`

Nitpick: One liner? if (this.shownStopwatch is null) return;

Nitpick: One liner? `if (this.shownStopwatch is null) return;`

Nitpick: newline

What's the purpose of this override?

Nitpick: newline

This is a great candidate for the new switch expression syntax, and then it can be turned into a lambda

private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch
{
    ToolTipIcon.Info => SystemIcons.Information.ToBitmap(),
    ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(),
    ToolTipIcon.Error => SystemIcons.Error.ToBitmap(),
    _ => null
}

This is a great candidate for the new `switch expression` syntax, and then it can be turned into a lambda ```csharp private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch { ToolTipIcon.Info => SystemIcons.Information.ToBitmap(), ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(), ToolTipIcon.Error => SystemIcons.Error.ToBitmap(), _ => null } ```

This is a great candidate for the new switch expression syntax, and then it can be turned into a lambda

private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch
{
    ToolTipIcon.Info => SystemIcons.Information.ToBitmap(),
    ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(),
    ToolTipIcon.Error => SystemIcons.Error.ToBitmap(),
    _ => null
}

This is a great candidate for the new `switch expression` syntax, and then it can be turned into a lambda ```csharp private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch { ToolTipIcon.Info => SystemIcons.Information.ToBitmap(), ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(), ToolTipIcon.Error => SystemIcons.Error.ToBitmap(), _ => null } ```

Nitpick: Lambda one liner

Nitpick: Naming convention is capitalised first letter

Nitpick: Naming convention is capitalised first letter title casing without underscores

Nitpick: convention is use of _ for private variables to avoid the use of this. Goes for all others in this section

This appears to be set but never read outside of the progress getter

This appears to be set but never read outside of the `progress` getter

This appears to be set but never read outside of the progress getter

This appears to be set but never read outside of the `progress` getter

Could this just read this.progressBar1.Value instead?

Could this just read `this.progressBar1.Value` instead?

Could this just read this.progressBar1.Value instead?

Could this just read `this.progressBar1.Value` instead?

Any reason we're wrapping this? Is it protected usually?

Does this need thises?

Does this need `this`es?

Does this need thises?

Does this need `this`es?

Nitpick: newline

Nitpick: Naming convention is capitalised first letter

Nitpick: timeout_ms should be timeoutMilliseconds

Nitpick: `timeout_ms` should be `timeoutMilliseconds`

Nitpick: timeout_ms should be timeoutMilliseconds

Nitpick: `timeout_ms` should be `timeoutMilliseconds`

immediate?

I try to rename my elements to match the TitleCase naming convention. Not a major one, just a nice-to-have. I appreciate this isn't consistent within the application anyway.

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

I'll change the comment to better explain the desired intent

            // don't animate, immediately "close"

rationale:
if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent ```suggestion // don't animate, immediately "close" ``` rationale: if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent

            // don't animate, immediately "close"

rationale:
if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent ```suggestion // don't animate, immediately "close" ``` rationale: if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I think this is just left over from my first attempts at getting the animation to work.

I went through and attempted to add a blank line before every if statement that was not the opening statement of the parent scope (and if preceded by a comment, the blank line went before the comment).

changed

I added a blank line before the if statement

removed

I added a blank line before the if statement

updated

swapped

removed

yes, I originally thought there would be more to the setter. Removed.

Not in the end. Removed. (I had originally though the UI would be .SetInfo(...); .Show() or something, but the way it is now has a better expandability and doesn't use this.

Not in the end. Removed. (I had originally though the UI would be `.SetInfo(...); .Show()` or something, but the way it is now has a better expandability and doesn't use this.

Not in the end. Removed. (I had originally though the UI would be .SetInfo(...); .Show() or something, but the way it is now has a better expandability and doesn't use this.

Not in the end. Removed. (I had originally though the UI would be `.SetInfo(...); .Show()` or something, but the way it is now has a better expandability and doesn't use this.

no, that's just what style I was used to back when I last worked in C# (removed all)

This got changed to timeoutSeconds to deal with corresponding comment in MainForm.cs

This got changed to `timeoutSeconds` to deal with corresponding comment in `MainForm.cs`

This got changed to timeoutSeconds to deal with corresponding comment in MainForm.cs

This got changed to `timeoutSeconds` to deal with corresponding comment in `MainForm.cs`

I added a blank line before the if statement

heh, wonder where that style came from...

settings dialog is using camelCase for everything....

I think the Pep8 guidance that "consistency within a file is most important" is a good point here.

I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

settings dialog is using camelCase for everything.... I think the Pep8 guidance that "consistency within a file is most important" is a good point here. I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

settings dialog is using camelCase for everything....

I think the Pep8 guidance that "consistency within a file is most important" is a good point here.

I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

settings dialog is using camelCase for everything.... I think the Pep8 guidance that "consistency within a file is most important" is a good point here. I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

renamed

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

> @alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style? So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

> @alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style? So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

I installed an ran the Rider ide and looked through for additional style changes (and applied the ones that didn't seem to conflict with the previous comments).

Nitpick: newline

Nitpick: One liner? if (this.shownStopwatch is null) return;

Nitpick: One liner? `if (this.shownStopwatch is null) return;`

Nitpick: newline

What's the purpose of this override?

Nitpick: newline

This is a great candidate for the new switch expression syntax, and then it can be turned into a lambda

private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch
{
    ToolTipIcon.Info => SystemIcons.Information.ToBitmap(),
    ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(),
    ToolTipIcon.Error => SystemIcons.Error.ToBitmap(),
    _ => null
}

This is a great candidate for the new `switch expression` syntax, and then it can be turned into a lambda ```csharp private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch { ToolTipIcon.Info => SystemIcons.Information.ToBitmap(), ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(), ToolTipIcon.Error => SystemIcons.Error.ToBitmap(), _ => null } ```

Nitpick: Lambda one liner

Nitpick: Naming convention is capitalised first letter

Nitpick: Naming convention is capitalised first letter title casing without underscores

Nitpick: convention is use of _ for private variables to avoid the use of this. Goes for all others in this section

This appears to be set but never read outside of the progress getter

This appears to be set but never read outside of the `progress` getter

Could this just read this.progressBar1.Value instead?

Could this just read `this.progressBar1.Value` instead?

Any reason we're wrapping this? Is it protected usually?

Does this need thises?

Does this need `this`es?

Nitpick: newline

Nitpick: Naming convention is capitalised first letter

Nitpick: timeout_ms should be timeoutMilliseconds

Nitpick: `timeout_ms` should be `timeoutMilliseconds`

immediate?

I try to rename my elements to match the TitleCase naming convention. Not a major one, just a nice-to-have. I appreciate this isn't consistent within the application anyway.

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

I'll change the comment to better explain the desired intent

            // don't animate, immediately "close"

rationale:
if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent ```suggestion // don't animate, immediately "close" ``` rationale: if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I think this is just left over from my first attempts at getting the animation to work.

I went through and attempted to add a blank line before every if statement that was not the opening statement of the parent scope (and if preceded by a comment, the blank line went before the comment).

changed

I added a blank line before the if statement

removed

I added a blank line before the if statement

updated

swapped

removed

yes, I originally thought there would be more to the setter. Removed.

Not in the end. Removed. (I had originally though the UI would be .SetInfo(...); .Show() or something, but the way it is now has a better expandability and doesn't use this.

Not in the end. Removed. (I had originally though the UI would be `.SetInfo(...); .Show()` or something, but the way it is now has a better expandability and doesn't use this.

no, that's just what style I was used to back when I last worked in C# (removed all)

This got changed to timeoutSeconds to deal with corresponding comment in MainForm.cs

This got changed to `timeoutSeconds` to deal with corresponding comment in `MainForm.cs`

I added a blank line before the if statement

heh, wonder where that style came from...

settings dialog is using camelCase for everything....

I think the Pep8 guidance that "consistency within a file is most important" is a good point here.

I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

settings dialog is using camelCase for everything.... I think the Pep8 guidance that "consistency within a file is most important" is a good point here. I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

renamed

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

> @alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style? So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

I installed an ran the Rider ide and looked through for additional style changes (and applied the ones that didn't seem to conflict with the previous comments).

Nitpick: newline

Nitpick: One liner? if (this.shownStopwatch is null) return;

Nitpick: One liner? `if (this.shownStopwatch is null) return;`

Nitpick: newline

What's the purpose of this override?

Nitpick: newline

This is a great candidate for the new switch expression syntax, and then it can be turned into a lambda

private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch
{
    ToolTipIcon.Info => SystemIcons.Information.ToBitmap(),
    ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(),
    ToolTipIcon.Error => SystemIcons.Error.ToBitmap(),
    _ => null
}

This is a great candidate for the new `switch expression` syntax, and then it can be turned into a lambda ```csharp private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch { ToolTipIcon.Info => SystemIcons.Information.ToBitmap(), ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(), ToolTipIcon.Error => SystemIcons.Error.ToBitmap(), _ => null } ```

Nitpick: Lambda one liner

Nitpick: Naming convention is capitalised first letter

Nitpick: Naming convention is capitalised first letter title casing without underscores

Nitpick: convention is use of _ for private variables to avoid the use of this. Goes for all others in this section

This appears to be set but never read outside of the progress getter

This appears to be set but never read outside of the `progress` getter

Could this just read this.progressBar1.Value instead?

Could this just read `this.progressBar1.Value` instead?

Any reason we're wrapping this? Is it protected usually?

Does this need thises?

Does this need `this`es?

Nitpick: newline

Nitpick: Naming convention is capitalised first letter

Nitpick: timeout_ms should be timeoutMilliseconds

Nitpick: `timeout_ms` should be `timeoutMilliseconds`

immediate?

I try to rename my elements to match the TitleCase naming convention. Not a major one, just a nice-to-have. I appreciate this isn't consistent within the application anyway.

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

I'll change the comment to better explain the desired intent

            // don't animate, immediately "close"

rationale:
if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent ```suggestion // don't animate, immediately "close" ``` rationale: if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I think this is just left over from my first attempts at getting the animation to work.

I went through and attempted to add a blank line before every if statement that was not the opening statement of the parent scope (and if preceded by a comment, the blank line went before the comment).

changed

I added a blank line before the if statement

removed

I added a blank line before the if statement

updated

swapped

removed

yes, I originally thought there would be more to the setter. Removed.

Not in the end. Removed. (I had originally though the UI would be .SetInfo(...); .Show() or something, but the way it is now has a better expandability and doesn't use this.

Not in the end. Removed. (I had originally though the UI would be `.SetInfo(...); .Show()` or something, but the way it is now has a better expandability and doesn't use this.

no, that's just what style I was used to back when I last worked in C# (removed all)

This got changed to timeoutSeconds to deal with corresponding comment in MainForm.cs

This got changed to `timeoutSeconds` to deal with corresponding comment in `MainForm.cs`

I added a blank line before the if statement

heh, wonder where that style came from...

settings dialog is using camelCase for everything....

I think the Pep8 guidance that "consistency within a file is most important" is a good point here.

I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

settings dialog is using camelCase for everything.... I think the Pep8 guidance that "consistency within a file is most important" is a good point here. I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

renamed

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

> @alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style? So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

I installed an ran the Rider ide and looked through for additional style changes (and applied the ones that didn't seem to conflict with the previous comments).

Nitpick: newline

Nitpick: One liner? if (this.shownStopwatch is null) return;

Nitpick: One liner? `if (this.shownStopwatch is null) return;`

Nitpick: newline

What's the purpose of this override?

Nitpick: newline

This is a great candidate for the new switch expression syntax, and then it can be turned into a lambda

private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch
{
    ToolTipIcon.Info => SystemIcons.Information.ToBitmap(),
    ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(),
    ToolTipIcon.Error => SystemIcons.Error.ToBitmap(),
    _ => null
}

This is a great candidate for the new `switch expression` syntax, and then it can be turned into a lambda ```csharp private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch { ToolTipIcon.Info => SystemIcons.Information.ToBitmap(), ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(), ToolTipIcon.Error => SystemIcons.Error.ToBitmap(), _ => null } ```

Nitpick: Lambda one liner

Nitpick: Naming convention is capitalised first letter

Nitpick: Naming convention is capitalised first letter title casing without underscores

Nitpick: convention is use of _ for private variables to avoid the use of this. Goes for all others in this section

This appears to be set but never read outside of the progress getter

This appears to be set but never read outside of the `progress` getter

Could this just read this.progressBar1.Value instead?

Could this just read `this.progressBar1.Value` instead?

Any reason we're wrapping this? Is it protected usually?

Does this need thises?

Does this need `this`es?

Nitpick: newline

Nitpick: Naming convention is capitalised first letter

Nitpick: timeout_ms should be timeoutMilliseconds

Nitpick: `timeout_ms` should be `timeoutMilliseconds`

immediate?

I try to rename my elements to match the TitleCase naming convention. Not a major one, just a nice-to-have. I appreciate this isn't consistent within the application anyway.

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

I'll change the comment to better explain the desired intent

            // don't animate, immediately "close"

rationale:
if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent ```suggestion // don't animate, immediately "close" ``` rationale: if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I think this is just left over from my first attempts at getting the animation to work.

I went through and attempted to add a blank line before every if statement that was not the opening statement of the parent scope (and if preceded by a comment, the blank line went before the comment).

changed

I added a blank line before the if statement

removed

I added a blank line before the if statement

updated

swapped

removed

yes, I originally thought there would be more to the setter. Removed.

Not in the end. Removed. (I had originally though the UI would be .SetInfo(...); .Show() or something, but the way it is now has a better expandability and doesn't use this.

Not in the end. Removed. (I had originally though the UI would be `.SetInfo(...); .Show()` or something, but the way it is now has a better expandability and doesn't use this.

no, that's just what style I was used to back when I last worked in C# (removed all)

This got changed to timeoutSeconds to deal with corresponding comment in MainForm.cs

This got changed to `timeoutSeconds` to deal with corresponding comment in `MainForm.cs`

I added a blank line before the if statement

heh, wonder where that style came from...

settings dialog is using camelCase for everything....

I think the Pep8 guidance that "consistency within a file is most important" is a good point here.

I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

settings dialog is using camelCase for everything.... I think the Pep8 guidance that "consistency within a file is most important" is a good point here. I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

renamed

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

> @alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style? So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

I installed an ran the Rider ide and looked through for additional style changes (and applied the ones that didn't seem to conflict with the previous comments).

Nitpick: newline

Nitpick: One liner? if (this.shownStopwatch is null) return;

Nitpick: One liner? `if (this.shownStopwatch is null) return;`

Nitpick: newline

What's the purpose of this override?

Nitpick: newline

This is a great candidate for the new switch expression syntax, and then it can be turned into a lambda

private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch
{
    ToolTipIcon.Info => SystemIcons.Information.ToBitmap(),
    ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(),
    ToolTipIcon.Error => SystemIcons.Error.ToBitmap(),
    _ => null
}

This is a great candidate for the new `switch expression` syntax, and then it can be turned into a lambda ```csharp private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch { ToolTipIcon.Info => SystemIcons.Information.ToBitmap(), ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(), ToolTipIcon.Error => SystemIcons.Error.ToBitmap(), _ => null } ```

Nitpick: Lambda one liner

Nitpick: Naming convention is capitalised first letter

Nitpick: Naming convention is capitalised first letter title casing without underscores

Nitpick: convention is use of _ for private variables to avoid the use of this. Goes for all others in this section

This appears to be set but never read outside of the progress getter

This appears to be set but never read outside of the `progress` getter

Could this just read this.progressBar1.Value instead?

Could this just read `this.progressBar1.Value` instead?

Any reason we're wrapping this? Is it protected usually?

Does this need thises?

Does this need `this`es?

Nitpick: newline

Nitpick: Naming convention is capitalised first letter

Nitpick: timeout_ms should be timeoutMilliseconds

Nitpick: `timeout_ms` should be `timeoutMilliseconds`

immediate?

I try to rename my elements to match the TitleCase naming convention. Not a major one, just a nice-to-have. I appreciate this isn't consistent within the application anyway.

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

I'll change the comment to better explain the desired intent

            // don't animate, immediately "close"

rationale:
if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent ```suggestion // don't animate, immediately "close" ``` rationale: if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I think this is just left over from my first attempts at getting the animation to work.

I went through and attempted to add a blank line before every if statement that was not the opening statement of the parent scope (and if preceded by a comment, the blank line went before the comment).

changed

I added a blank line before the if statement

removed

I added a blank line before the if statement

updated

swapped

removed

yes, I originally thought there would be more to the setter. Removed.

Not in the end. Removed. (I had originally though the UI would be .SetInfo(...); .Show() or something, but the way it is now has a better expandability and doesn't use this.

Not in the end. Removed. (I had originally though the UI would be `.SetInfo(...); .Show()` or something, but the way it is now has a better expandability and doesn't use this.

no, that's just what style I was used to back when I last worked in C# (removed all)

This got changed to timeoutSeconds to deal with corresponding comment in MainForm.cs

This got changed to `timeoutSeconds` to deal with corresponding comment in `MainForm.cs`

I added a blank line before the if statement

heh, wonder where that style came from...

settings dialog is using camelCase for everything....

I think the Pep8 guidance that "consistency within a file is most important" is a good point here.

I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

settings dialog is using camelCase for everything.... I think the Pep8 guidance that "consistency within a file is most important" is a good point here. I opted to make the new things in SettingsDialogue always use camelCase, but changed the NotificationDialogue be TitleCase.

renamed

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

> @alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style? So typically I use Rider (alternatively Resharper) whilst working and it picks out most of these things. As far as I know, what it works on is the microsoft standard convention, so I just use that as it's pretty universal and sensible.

I installed an ran the Rider ide and looked through for additional style changes (and applied the ones that didn't seem to conflict with the previous comments).

Nitpick: newline

Nitpick: One liner? if (this.shownStopwatch is null) return;

Nitpick: One liner? `if (this.shownStopwatch is null) return;`

Nitpick: One liner? if (this.shownStopwatch is null) return;

Nitpick: One liner? `if (this.shownStopwatch is null) return;`

Nitpick: newline

What's the purpose of this override?

Nitpick: newline

This is a great candidate for the new switch expression syntax, and then it can be turned into a lambda

private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch
{
    ToolTipIcon.Info => SystemIcons.Information.ToBitmap(),
    ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(),
    ToolTipIcon.Error => SystemIcons.Error.ToBitmap(),
    _ => null
}

This is a great candidate for the new `switch expression` syntax, and then it can be turned into a lambda ```csharp private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch { ToolTipIcon.Info => SystemIcons.Information.ToBitmap(), ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(), ToolTipIcon.Error => SystemIcons.Error.ToBitmap(), _ => null } ```

This is a great candidate for the new switch expression syntax, and then it can be turned into a lambda

private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch
{
    ToolTipIcon.Info => SystemIcons.Information.ToBitmap(),
    ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(),
    ToolTipIcon.Error => SystemIcons.Error.ToBitmap(),
    _ => null
}

This is a great candidate for the new `switch expression` syntax, and then it can be turned into a lambda ```csharp private Image? ConvertToolTipIconToImage(ToolTipIcon icon) => icon switch { ToolTipIcon.Info => SystemIcons.Information.ToBitmap(), ToolTipIcon.Warning => SystemIcons.Warning.ToBitmap(), ToolTipIcon.Error => SystemIcons.Error.ToBitmap(), _ => null } ```

Nitpick: Lambda one liner

Nitpick: Naming convention is capitalised first letter

Nitpick: Naming convention is capitalised first letter title casing without underscores

Nitpick: convention is use of _ for private variables to avoid the use of this. Goes for all others in this section

This appears to be set but never read outside of the progress getter

This appears to be set but never read outside of the `progress` getter

This appears to be set but never read outside of the progress getter

This appears to be set but never read outside of the `progress` getter

Could this just read this.progressBar1.Value instead?

Could this just read `this.progressBar1.Value` instead?

Could this just read this.progressBar1.Value instead?

Could this just read `this.progressBar1.Value` instead?

Any reason we're wrapping this? Is it protected usually?

Does this need thises?

Does this need `this`es?

Does this need thises?

Does this need `this`es?

Nitpick: newline

Nitpick: Naming convention is capitalised first letter

Nitpick: timeout_ms should be timeoutMilliseconds

Nitpick: `timeout_ms` should be `timeoutMilliseconds`

Nitpick: timeout_ms should be timeoutMilliseconds

Nitpick: `timeout_ms` should be `timeoutMilliseconds`

immediate?

I try to rename my elements to match the TitleCase naming convention. Not a major one, just a nice-to-have. I appreciate this isn't consistent within the application anyway.

@alexhorner, is there a linter tool or editor config file that I can setup to check code against the desired style?

I'll change the comment to better explain the desired intent

            // don't animate, immediately "close"

rationale:
if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent ```suggestion // don't animate, immediately "close" ``` rationale: if a user click the x button, they expect the window to go away "now". My observation is that general Windows users are about 50% split on whether a going away animation in response to telling a window to close is annoying or "fine" (and only a few people "prefer it"). Because of this, I intentionally implemented the button close handler to immediately close the pop up (don't animate, and don't wait for the timeout).

I'll change the comment to better explain the desired intent

            // don't animate, immediately "close"