SaveChanges: Database.SetCommandTimeout overload that accepts a TimeSpan argument

[MicahZoltu]

Int32 is ambiguous (milliseconds, seconds, minutes?) and shouldn't be used as a type when you really want a duration. Instead, TimeSpan should be used.

https://github.com/aspnet/EntityFramework/blob/master/src/Microsoft.EntityFrameworkCore.Relational/RelationalDatabaseFacadeExtensions.cs#L156

I don't mind submitting a PR for this, though I'm not sure what the proper deprecation strategy is for this project. Create an overload and somehow annotate the old one? Leave the old one and new one side-by-side (personally not a fan of this one)? A doc comment on the old one indicating it is deprecated? Delete the old one and accept the breaking change?

https://github.com/aspnet/EntityFramework/blob/master/src/Microsoft.EntityFrameworkCore.Relational/RelationalDatabaseFacadeExtensions.cs#L159 should similarly return a TimeSpan, not an Int32.

[rowanmiller]

Hey,

We'd be happy to take a PR that adds an overload that takes TimeSpan. Specifying timeout in int is how this is handled in ADO.NET - which is why we have that API. But we agree that having the ability to specify in TimeSpan would make code clearer.

~Rowan

[divega]

I think XML docs could help here.

[rowanmiller]

Agreed, this is one of the parts of the product that hasn't been doc'd yet

[rowanmiller]

Closing this out as we aren't planning to make the change, but feel free to send a PR for it