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

Add MQTTnet package #258

Closed
wants to merge 5 commits into from
Closed

Add MQTTnet package #258

wants to merge 5 commits into from

Conversation

TapioRantala
Copy link
Contributor

@TapioRantala TapioRantala commented Oct 6, 2023

The NuGet package needs to respect a few constraints in order to be listed in the curated list:

  • Add a link to the NuGet package: https://www.nuget.org/packages/MQTTnet/
  • It must have non-preview versions (e.g 1.0.0 but not 1.0.0-preview.1)
  • It must provide .NETStandard2.0 assemblies as part of its package
  • The lowest version added must be the lowest .NETStandard2.0 version available
  • The package has been tested with the Unity editor
  • The package has been tested with a Unity standalone player
    • if the package is not compatible with standalone player, please add a comment to a Known issues section to the top level readme.md
  • All package dependencies with .NETStandard 2.0 target must be added to the PR (respecting the same rules above)
    • Note that if a future version of the package adds a new dependency, this dependency will have to be added manually as well

Note: The server will be updated only when a new version tag is pushed on the main branch, not necessarily after merging this pull-request.

The version is not the first version available on .NETStandard2.0, that would be 2.5.1.
There are additional dependencies listed for versions before 3.0.15:
NETStandard.Library (>= 2.0.0)
System.Net.Security (>= 4.3.2)
System.Net.WebSockets (>= 4.3.0)
System.Net.WebSockets.Client (>= 4.3.1)
System.Threading.Thread (>= 4.3.0)
These look like they are part of the .NETStandard2.0 itself?

Not sure if I should add those with "ignore": true or if having not-lowest-version is ok.

@bdovaz
Copy link
Collaborator

bdovaz commented Oct 6, 2023

@TapioRantala

2023-10-06T12:33:32.4929492Z
2023-10-06T12:33:40.7789869Z Failed Ensure_Min_Version_Is_Correct_Ignoring_Analyzers_And_Native_Libs [11 s]
2023-10-06T12:33:40.7799156Z Error Message:
2023-10-06T12:33:40.7805032Z Package MQTTnet
2023-10-06T12:33:40.7813637Z Expected: <2.5.1>

@bdovaz
Copy link
Collaborator

bdovaz commented Oct 9, 2023

@TapioRantala please, run tests locally

Failed Ensure_That_Packages_Already_Included_In_Net_Standard_Are_not_Included_In_The_Registry [140 ms]
2023-10-09T12:19:10.5023362Z Error Message:
2023-10-09T12:19:10.5029297Z Expected:
2023-10-09T12:19:10.5032267Z But was: < "System.Net.Security", "System.Net.WebSockets", "System.Threading.Thread" >
2023-10-09T12:19:10.5032694Z
2023-10-09T12:19:10.5032811Z Stack Trace:
2023-10-09T12:19:10.5033519Z at UnityNuGet.Tests.RegistryTests.Ensure_That_Packages_Already_Included_In_Net_Standard_Are_not_Included_In_The_Registry() in D:\a\UnityNuGet\UnityNuGet\src\UnityNuGet.Tests\RegistryTests.cs:line 3

Copy link

This pull request has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this pull request should stay open, please remove the Stale label or comment on the pull request.

@github-actions github-actions bot added the Stale label Nov 19, 2023
@TapioRantala
Copy link
Contributor Author

TapioRantala commented Nov 20, 2023

I believe the concerns have been addressed now and this is ready to be merged.

@bdovaz
Copy link
Collaborator

bdovaz commented Nov 20, 2023

Please, check comment @TapioRantala

@github-actions github-actions bot removed the Stale label Nov 21, 2023
@TapioRantala
Copy link
Contributor Author

Tests are passing, please clarify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants