Further cleanup/improvements #269
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This enables nullable annotations in the projects and adjust all types accordingly. There are no warnings left, everything should be good to go.
This enables nullable annotations in the projects and adjusts all types accordingly. There are three unresolved warnings due to properties that are assigned to non-nullable values when assigned, but have no default value. This can be resolved by the "required" member attribute, but this is only available with C# 11 (.NET 7). Keep the warnings for now. Note that the Nullable definition is moved to Directory.Build.props now, since we have it enabled in all core projects now.
Use "Get" and "replace" which are the defaults for unobtrusive ajax here as well. This removes the nullable warning for HttpMethod.
Not really much to do here either. While at it, use explicit types on some int variables.
* Use more specific Assert methods * Use "void" return type where no async/await is used * Remove unused local variables
This has an async-over-sync implementation, but no native sync one. Add it.
At the moment, Extensions like ToPagedList and ToPagedListAsync are defined
in the same namespace as all the other types. This poses a few problems:
* If one wants to use X.PagedList.EF, he will have name collisions between
the EF version of ToPagedListAsync and the default async-over-sync version.
While this can be resolved by choosing the X.PagedList.EF namespace in a
"using", using any of the base types like IPagedList would still require
to add the X.PagedList "using" again or use a full qualifier.
* If somebody only wants the base types like IPagedList, but desires to
implement Extensions on his own, he will draw back in the upstream
Extensions by a "using" statement.
* If somebody wants to hide the async-over-sync implementations from
inexperienced developers, but needs the rest of the package, there is no
option so far.
All of this can be resolved by putting the different Extensions in their
dedicated namespaces. This way one can choose whether he only wants the base
classes and interfaces or whether different types of Extensions are desired.
Choosing between them is as simple as putting the appropriate "using" on top
of the file. This allows to switch between the EF and async-over-sync
implementations as it does allow to disable some of them.
The new namespaces are:
X.PagedList.Extensions The sync implementations
X.PagedList.AsyncExtensions The fake async (async-over-sync) implementations
X.PagedList.EF The proper async EF Core implementations
(this namespace is unchanged)
This will require users of earlier package versions to add them too, of course,
but this is trivial.
This was referenced Jul 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is based on the changes in #268
If you want both, just take this PR.