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.

Sign up for GitHub

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

Jump to bottom

#11304 Add additional TFM's and optimise dependencies #11306

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

#11304 Add additional TFM's and optimise dependencies #11306

wants to merge 1 commit into from

Conversation

thompson-tomo
Copy link

@thompson-tomo thompson-tomo commented Apr 6, 2024 •
edited

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

  • There is a single JIRA ticket related to the pull request.
  • The web-link to the pull request is attached to the JIRA ticket.
  • The JIRA ticket has the Patch Available state.
  • The pull request body describes changes that have been made.
    The description explains WHAT and WHY was made instead of HOW.
  • The pull request title is treated as the final commit message.
    The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
  • A reviewer has been mentioned through the JIRA comments
    (see the Maintainers list)
  • The pull request has been checked by the Teamcity Bot and
    the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

closes #11304

@ptupitsyn
Copy link
Contributor

@thompson-tomo
Could you please explain what are we trying to fix here? I don't think there is any problem with the current state of things. We build for netstandard2.0, and the resulting package can be used from any supported .NET version.

@thompson-tomo
Copy link
Author

By adding net standard 2.1 we are able to remove 2 dependencies which could result in conflicts. By adding the second TFM a further dependency can be removed eliminating further chance of conflict & reducing deployment size + chance of vulnerabilities.

@ptupitsyn
Copy link
Contributor

ptupitsyn commented Apr 9, 2024 •
edited

which could result in conflicts

The dependencies specify a minimum version (>= 4.7.0), not a specific one, so there should be no conflicts.

reducing deployment size

It won't change. To verify:

  • Create a new .NET 8 project: dotnet new console
  • Add a reference to Apache.Ignite package: dotnet add package Apache.Ignite
  • Publish: dotnet publish -c Release -o pub
  • Check the resulting pub folder - it does not contain System.Reflection.Emit or Microsoft.Win32.Registry, because those come with .NET 8 installation

Am I missing something? Do you have a real-world use case where those dependencies are causing issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove polyfill packages
2 participants
@thompson-tomo @ptupitsyn