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

Platform-Specific Enhancements #322

Closed
shiftkey opened this issue Jan 22, 2014 · 9 comments
Closed

Platform-Specific Enhancements #322

shiftkey opened this issue Jan 22, 2014 · 9 comments

Comments

@shiftkey
Copy link
Member

So this neat little snippet came across my twitter stream today from @nigel-sampson:

https://gist.github.com/nigel-sampson/8566723

It's a way to do OAuth inside a Win8 app.

While we're transitioning to PCLs to replace our existing .net45 and .netcore45 projects, I'm curious if people are interested in contributing enhancements like this which aren't necessarily part of the core.

Thoughts?

@haacked
Copy link
Contributor

haacked commented Jan 22, 2014

We have a precedence for little enhancements like this: https://github.com/octokit/octokit.net/blob/master/Octokit.Reactive/Helpers/AuthorizationExtensions.cs

So I'm not opposed to it if it feels core. Like it's a very very common scenario. But if we start to see too many of these, we may want to consider another wrapper library. But for now, I wouldn't be opposed to putting something like this in Octokit.csproj.

@shiftkey
Copy link
Member Author

Extensions are fine when they work across all the platforms supported.

The problem with the snippet above is that it depends on WebAuthenticationBroker which is Win8-specific. Moving to PCLs will kill off our ability to #ifdef in functionality like this at all.

@haacked
Copy link
Contributor

haacked commented Jan 22, 2014

Ah, so this has to be in a separate Octokit.Win8 library. But correct me if I'm wrong, is the actual behavior Win 8 specific? Or does it just happen to use Win8 specific libraries but could be generalized?

@shiftkey
Copy link
Member Author

@haacked showing a browser and getting some data from the result of the user's interaction is what the class does - we could hide that behind an abstraction (I know @dsplaisted has shown how to do that with PCLs before) but it then means we need to write a similar abstraction for desktop.

In the short term perhaps shipping a Octokit.WindowsRuntime project might be easier - when multiple platforms want it, then we revisit generalizing the feature...

@haacked
Copy link
Contributor

haacked commented Jan 22, 2014

showing a browser and getting some data from the result of the user's interaction is what the class does

Ah, that seems important since that's the preferred approach for OAuth when it comes to 3rd party apps. So if we did that generically, I'd want it in Octokit core. I'm wondering if this Octokit.WindowsRuntime library is important enough for us to bother with it. Is there a demand?

@shiftkey
Copy link
Member Author

Is there a demand?

I'll leave it up to everyone else. I'm just trying to think a couple of steps ahead...

@haacked
Copy link
Contributor

haacked commented Jan 23, 2014

Look at you, always skating to where the puck is going.

@dsplaisted
Copy link
Contributor

I don't think that you absolutely have to supply an implementation of the abstraction on all the platforms you support. On other platforms you can require the user to supply the implementation of the abstraction if needed. Certainly it would be nice if you did it for all platforms, but better to have an in-the-box implementation in just WinStore than none at all.

I think it would be great to have a library that supplied a portable way of doing OAuth. Maybe this could go in https://github.com/paulcbetts/splat, or it could be its own thing.

Here's a great post on how to deal with this kind of stuff: http://blog.stephencleary.com/2012/11/portable-class-library-enlightenment.html

@shiftkey
Copy link
Member Author

It's been a while since this thread last had any activity.

I think the push towards doing more with HttpMessageHandler that's available to the consumer to craft and control means we can keep the library simpler.

Please chime in over on #781 if you have any ideas for scenarios we should keep in mind.

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

No branches or pull requests

3 participants