-
Notifications
You must be signed in to change notification settings - Fork 189
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
refactor: Use netstandard2.0
as target framework to support wider frameworks
#845
Conversation
netstandard2.0
as target framework to support wider frameworksnetstandard2.0
as target framework to support wider frameworks
@@ -1,14 +1,14 @@ | |||
<Project Sdk="Microsoft.NET.Sdk"> | |||
<PropertyGroup> | |||
<TargetFrameworks>net48;net6.0</TargetFrameworks> | |||
<PackageLicenseFile>LICENSE.txt</PackageLicenseFile> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed and added netstandard2.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you mean licence.txt
, then it was duplicated.
<GenerateDocumentationFile>true</GenerateDocumentationFile> | ||
</PropertyGroup> | ||
<PropertyGroup> | ||
<LangVersion>latest</LangVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be we keep it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed here, added upper.
#if !NET48 | ||
private HttpClient CreateHttpClientInstance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nvborisenko have you run some tests against .NET framework 4.7+?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know any known issues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only .net framework 4.7 is under interest? netstandard2.0
is supported by .net framework 4.6.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You said you were trying to run with. Net 4.7, so this is why I asked.
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know any known issues?
Should be the same issues I had for 4.8 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not from Selenium team, I am contributing as personal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And you will be surprised if you look at dependencies of Playwright.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Let's go ahead with this change.
Fingers crossed :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Selenium is already felt it.
Hi @nvborisenko, congrats, the Appium project wants to compensate you for this contribution! Please reply to this comment mentioning me and sharing your OpenCollective account name, so that we can initiate payment! Or let me know if you decline to receive compensation via OpenCollective. Thank you! |
List of changes
Currently Appium targets to support:
It means that I cannot use Appium with .net framework 4.7
Types of changes
Use
netstandard2.0
to cover more frameworks.https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-0#when-to-target-net80-vs-netstandard
Documentation
This can be done by navigating to the documentation section on http://appium.io selecting the appropriate command/endpoint and clicking the 'Edit this doc' link to update the C# example
Integration tests
Details
Please provide more details about changes if necessary. You can provide code samples showing how they work and possible use cases if there are new features. Also, you can create gists with pasted C# code samples or put them here using markdown.
About markdown please read Mastering markdown and Writing on GitHub