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

Feat .NET Server SDK updates #320

Merged
merged 83 commits into from
May 14, 2023
Merged

Conversation

abnegate
Copy link
Contributor

@abnegate abnegate commented Nov 29, 2021

Up to date for 1.3.x

  • Support for all targets from .NET Framework 4.6.1 onward
  • Response models
  • Chunked upload
  • Resumable upload
  • InputFile uploads
  • Permission, Role, ID, Query helpers

…e-models

# Conflicts:
#	templates/dotnet/docs/example.md.twig
#	templates/dotnet/src/Appwrite/Services/ServiceTemplate.cs.twig
@abnegate abnegate requested a review from eldadfux January 11, 2023 04:32
@abnegate
Copy link
Contributor Author

@eldadfux @lohanidamodar Would love to get this merged soon if we can 🙏

@lohanidamodar
Copy link
Member

@abnegate can you get this synced with latest changes?

@abnegate
Copy link
Contributor Author

@lohanidamodar Updated, only Deno tests still failing

@pietrodicaprio
Copy link

pietrodicaprio commented Feb 18, 2023

Hi, I saw this PR only after troubleshooting the reason for the currently published NuGet package not working.
I built a version of the SDK with minimal changes just to have it properly generated and working, there is a draft PR #617

Just 2 cents and genuine questions

I see a big effort ongoing here but also some changes that get me ask "why?". For example: the greatest feature of Net Standard is the compatibility with both Net Framework and Net Core, why having so many targets in the project when netstandard2.0 gives you the best compatibility with any Framework that is not End Of Life?
The oldest EOL framework is 4.5.1 that should not be used for new projects nor for old Appwrite projects since they would be still newer than NF 4.5.1. Moreover keeping only one target reduces the requirements for the build which is reduced to Net Core 3.0 instead of both at least NC 3.0 and NF 4.8 allowing better and easier CI.

I see both great things and not-so-great things in a huge amount of commits going back to October 2021, generating a new question: if the currently published NuGet package does not work, why not updating the SDK generator a bit at a time? At least the updates are more under control.

@abnegate
Copy link
Contributor Author

abnegate commented Mar 24, 2023

Hey @pietrodicaprio 👋 the many platforms were added to support developers who might not be able to use the latest platforms for whatever reason.

In terms of size - this PR has certainly grown a lot over time! It was initially intended as an incremental update, but as new features were added in new releases of Appwrite, those new features had to be supported so this PR could be merged.

Hoping to get this merged soon 🤞

@pietrodicaprio
Copy link

@abnegate but a csproj like that would require the dev to have multiple SDKs installed. Would be easier to put one and let the dev change it to whatever has available when tries to build.
The nuget package shuld be based only on netstandard2.0 instead that provides the most (not outdated and not supported) compatibility.

@adityaoberai
Copy link
Member

adityaoberai commented Mar 24, 2023

@abnegate but a csproj like that would require the dev to have multiple SDKs installed. Would be easier to put one and let the dev change it to whatever has available when tries to build. The nuget package shuld be based only on netstandard2.0 instead that provides the most (not outdated and not supported) compatibility.

@abnegate just double checking, do we have any framework or platform specific code in the Appwrite .NET SDK that requires us to multi-target?

Also, are we targetting any particular .NET version that needs us to target netstandard1.3? (especially since the recommendation is to not do so otherwise)

@abnegate
Copy link
Contributor Author

abnegate commented May 3, 2023

@pietrodicaprio @adityaoberai I've updated the target frameworks based on the current Microsoft recommendations 👍

@abnegate abnegate requested review from adityaoberai and removed request for TorstenDittmann, christyjacob4 and eldadfux May 3, 2023 02:13
@lohanidamodar
Copy link
Member

@abnegate please fix the conflict and I'll merge once @adityaoberai is able to review.

@adityaoberai
Copy link
Member

@abnegate please fix the conflict and I'll merge once @adityaoberai is able to review.

I looked at the updates and so far, it all looks good. One thing though, this would have to incorporate the fix for chunked uploads made in #648 as well, right?

…e-models

# Conflicts:
#	templates/dotnet/docs/example.md.twig
@lohanidamodar lohanidamodar merged commit 6d747e4 into master May 14, 2023
@lohanidamodar lohanidamodar deleted the feat-dotnet-response-models branch May 14, 2023 05:57
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.

🚀 Feature: Support GA Release
5 participants