Update facebook API version to 2.12 #1673
Conversation
| @@ -585,7 +585,7 @@ private void ConfigureDefaults(FacebookOptions o) | |||
| var transaction = await server.SendAsync("http://example.com/base/login"); | |||
| Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); | |||
| var location = transaction.Response.Headers.Location.AbsoluteUri; | |||
| Assert.Contains("https://www.facebook.com/v2.6/dialog/oauth", location); | |||
| Assert.Contains("https://www.facebook.com/v2.12/dialog/oauth", location); | |||
There was a problem hiding this comment.
I could go either way on that. There are pros/cons to using the fields vs. repeating the data.
There was a problem hiding this comment.
Yeah but in this case isn't it basically always a bug if these ever are inconsistent since we are exposing this in our Defaults class?
There was a problem hiding this comment.
Pros to using const's:
- Less work to maintain because you make the change in one place
Cons to using const's:
- Less clear when you make a src change to see what effect it has on the product
So, yes, it'd be a bug if it's inconsistent, and this test would tell you that, and you have to fix it (of course). Using a const would avoid the need to have to change the test.
There was a problem hiding this comment.
But mainly the upside is it makes future updates trivial, as its just updating the constant
There was a problem hiding this comment.
And that's why I'm fine either way 😄
There was a problem hiding this comment.
I actually prefer updating it separately in the tests. It's like having to enter your e-mail address twice to confirm you did it right.
#1306 No behavioral changes.
Our current version (2.6) will expire July 13, 2018.