Skip to content

Conversation

hermitdave
Copy link
Contributor

Expose ConnectionType property
Rework IsInternetAvailable property

Work on #455

Expose ConnectionType
Rework IsInternetAvailable property
@dnfclas
Copy link

dnfclas commented Oct 12, 2016

Hi @hermitdave, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla2.dotnetfoundation.org.

TTYL, DNFBOT;

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good so far, I think the ConnectionHelper class now helps in most of the cases 👍

/// <summary>
/// No network connection - offline
/// </summary>
///
Copy link

Choose a reason for hiding this comment

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

This line can be removed.

</None>
<None Include="project.json" />
<None Include="stylecop.json" />
<None Include="stylecop1.json" />
Copy link

@ghost ghost Oct 12, 2016

Choose a reason for hiding this comment

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

I think the stylecop.json files don't belong to this PR. Please remove the lines from the project file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jlnostr done.. added those by mistake.. removed and pushed

Removed Offline ConnectionType
Defaulting to Unknown ConnectionType
@deltakosh
Copy link
Contributor

Do you mind signing the new DNF CLA?

@dnfclas
Copy link

dnfclas commented Oct 13, 2016

@hermitdave, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@deltakosh
Copy link
Contributor

There is always UWP alternative as we are only using UWP code ;)
But the point is all about simplicity. If it is as simple as this helper then we should not merge the PR.

But here I think the helper is clearly about centralizing things that could be on NetworkInformation or ConnectionProfile. This is not rocket science but more a convenient layer on top of the SDK (a lot of our helpers are like this)

@ScottIsAFool
Copy link
Contributor

@ThreeFive-O I think the purpose of this is to offer explicit properties so that the dev just has one call to make, rather than several.

@ThreeFive-O
Copy link

Yeah, I realized when looking more closely at the code. This is why I deleted my comment, but then again, the word was already out :D Pardon me!

@deltakosh
Copy link
Contributor

Please don't apologize. This is a community project. We all need to discuss. We cannot have a single opinion. This is great.

@deltakosh deltakosh merged commit 2e486c9 into dev Oct 17, 2016
@deltakosh deltakosh deleted the HD-NetworkHelper branch October 17, 2016 21:02
@deltakosh deltakosh restored the HD-NetworkHelper branch October 17, 2016 21:02
@hermitdave hermitdave deleted the HD-NetworkHelper branch October 18, 2016 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants