Skip to content

Review public API surface #734

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

Closed
13 tasks done
bart-degreed opened this issue May 8, 2020 · 2 comments · Fixed by #808
Closed
13 tasks done

Review public API surface #734

bart-degreed opened this issue May 8, 2020 · 2 comments · Fixed by #808
Labels

Comments

@bart-degreed
Copy link
Contributor

bart-degreed commented May 8, 2020

Things I'd like to change:

  • Rename 'entity' parameters to 'resource' in DefaultResourceService
  • Rename DefaultResourceRepository to EntityFrameworkCoreRepository (and update variable/parameter names likewise)
  • Move all types required for basic usage into JADNC's non-internal namespace. Advanced usage, like custom Service/Repository/ResourceDefinition and query filters can be in Internal namespaces. This should clarify our intent on taking breaking changes.
  • Make options.DefaultPageSize nullable (disallow setting to 0)
  • Rename T in JsonApiController<T> to TResource
  • Rename IQueryParameterParser to IQueryStringParser (it parses all types, not a single one)
  • Replace IEnumerable<> / List<> with IReadOnlyCollection<>
  • Review null checks
  • Validate input parameters on public members, validate options.
  • Rename CurrentRequest to JsonApiRequest

Other

@maurei
Copy link
Member

maurei commented May 15, 2020

Rename DefaultResourceRepository to DefaultEntityRepository (and update
variable/parameter names likewise)

What would be the motivation for this?

@bart-degreed
Copy link
Contributor Author

It is an implementation of a resource repository that uses ORM entities. Other implementations could be named AmazonResourceService, CachingRedisRepository etc.

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

Successfully merging a pull request may close this issue.

2 participants