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

refactor: Use netstandard2.0 as target framework to support wider frameworks #845

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

nvborisenko
Copy link
Contributor

List of changes

Currently Appium targets to support:

  • net6.0
  • net framework 4.8

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

For existing code that targets .NET Standard 2.0 or later, there's no need to change the TFM to net8.0 or a later TFM. The only reason to retarget from .NET Standard to .NET 8+ would be to gain access to more runtime features, language features, or APIs.

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change that adds functionality or value)
  • Breaking change (fix or feature that would cause existing functionality not to work as expected)
  • Test fix (non-breaking change that improves test stability or correctness)

Documentation

  • Have you proposed a file change/ PR with Appium to update 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

  • Have you provided integration tests for your changes? (required for Bugfix, New feature, or Test fix)

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

@Dor-bl Dor-bl changed the title Use netstandard2.0 as target framework to support wider frameworks refactor: Use netstandard2.0 as target framework to support wider frameworks Oct 18, 2024
@@ -1,14 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net48;net6.0</TargetFrameworks>
<PackageLicenseFile>LICENSE.txt</PackageLicenseFile>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor Author

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

Copy link
Contributor Author

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>
Copy link
Collaborator

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed here, added upper.

Comment on lines -73 to -74
#if !NET48
private HttpClient CreateHttpClientInstance
Copy link
Collaborator

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+?

Copy link
Contributor Author

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?

Copy link
Contributor Author

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

Copy link
Collaborator

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.
:)

Copy link
Collaborator

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 😁

Copy link
Contributor Author

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As for Selenium:

image

Copy link
Contributor Author

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.

Copy link
Collaborator

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 :)

Copy link
Contributor Author

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.

@Dor-bl Dor-bl added Enhancement .NET Pull requests that update .net code labels Oct 18, 2024
@Dor-bl Dor-bl merged commit 4330885 into appium:main Oct 18, 2024
3 checks passed
@nvborisenko nvborisenko deleted the netstandard branch October 20, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement .NET Pull requests that update .net code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants