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

Feedback #261

Open
NecatiMeral opened this issue Apr 27, 2020 · 2 comments
Open

Feedback #261

NecatiMeral opened this issue Apr 27, 2020 · 2 comments

Comments

@NecatiMeral
Copy link

NecatiMeral commented Apr 27, 2020

Hi,

thanks for maintaining this project.
I'm using your project in a small bridge project between different systems.

Maybe you want to improve the usability of the library, so here's my feedback after working with it for two days:

  1. The first thing I noticed are some inconsistencies. For example, I want to create a TimeEntry, so I'm loading the required TimeEntryActivity for the Activity property. But wait, I can`t assign my retreived activity since there are some type shenanigans going on. Related to Version 3.0.6.1 makes IssueCustomField.Info readonly breaking existing usage #253
  2. Not all redmine APIs support filtering, so there should be a built-in GetObjects<T>()-method (without the NameValueCollection parameter. Or even better, create individual services per type, to support individual queries (GetProjectsByIdOrIdentifierAsync(string id) for example).
  3. You should get rid of controlling the serialization and deserialization of models inside the models. Why don't you just create a ISerializer interface and implement it per XML and JSON? Because of the limited object (de)serialization capabilities, I cannot serialize the object by myself to cache them. For now, I sticked with custom dtos to make them cacheable properly.
  4. Consider separating your classes by areas and not by types. imho the API feels more organized if I access a service or model by it's area: Redmine.Net.Issues.Issue instead of Redmine.Net.Types.Issue (i'm thinking about something like Redmine.Net.Issues.IRedmineIssueClient, so you have all related models in the same namespace.
  5. User impersonation should provide a limited scope. Maybe something like:
using(var apiAsUser = redmineManager.ImpersonateUser("loginname"))
{
    // do something
}
  1. The project itself feels a little bit "sluggigh" or instable.
@zapadi
Copy link
Owner

zapadi commented Apr 28, 2020

Thank you for your feedback.

I think it will be great if you will open a pull request with all of your suggestions in order to improve the library.

@NecatiMeral
Copy link
Author

Hi @zapadi,

I think my interpretation of the library would be a mess of breaking changes, so users wouldn't be happy about it.
But I'll think about it.

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

No branches or pull requests

2 participants