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

What do you think of extracting TypeParameterResolver into its own module? #2746

Closed
vlsi opened this issue Nov 28, 2022 · 10 comments
Closed

Comments

@vlsi
Copy link

vlsi commented Nov 28, 2022

TypeParameterResolver looks like a nice standalone API, which is reusable even without mybatis.
Have you considered splitting mybatis-3 jar into several modules, so TypeParameterResolver could be reused across projects?

@harawata
Copy link
Member

I had that usage in mind when writing the class, however, the class is actually "optimized" for MyBatis. In other words, it may not be able to handle some complex cases that are not necessary in MyBatis (I cannot recall the details off the top of my head, though) and I prefer to keep the class as simple as possible in this repo.

I'm still OK with the idea of maintaining/releasing the class as a separate library/module if you think there is a demand.

@vlsi
Copy link
Author

vlsi commented Nov 28, 2022

I applied TypeParameterResolver when implementing Guice's TypeListener, so I could get method return type with account of actual type parameters. It did work like a charm.

It is unfortunate Guice has no machinery like that, and I am afraid they will be really slow to add APIs (as you might know, Guice releases are really rare).

I found mybatis by running global search on GitHub for getActualTypeArguments.

I agree "reflection utils" might be not the primary goal for mybatis, however, TypeParameterResolver would definitely be useful outside of mybatis even with all its limitations.

@harawata
Copy link
Member

I was wondering how you found the class. :D

I may be able to set up a new repo (probably in my personal account) and release the first version this weekend.
If you are willing to maintain it under your own repo, I'm totally fine with it as long as it follows the Apache License 2.

@vlsi
Copy link
Author

vlsi commented Nov 28, 2022

If you are willing to maintain it under your own repo, I'm totally fine with it as long as it follows the Apache License 2.

I just thought having a module under the existing mybatis groupid id would be fine, and I thought it would be easier for you (e.g. the lib could have the same version number as mybatis itself).

It might be integrating/requesting the API from https://github.com/classgraph/classgraph might be a slightly better idea than creating a brand-new top-level library.

@harawata
Copy link
Member

mybatis products won't depend on the new module (or vice versa), so it does not seem appropriate to keep it under mybatis org.
We only have a few active devs in this org and there already are many repos to maintain, you know.

It might be integrating/requesting the API from https://github.com/classgraph/classgraph might be a slightly better idea than creating a brand-new top-level library.

I am not sure how this works, but if you could take care of it, I am OK, I think.
This means that there is nothing to do on my end, is that correct?

@vlsi
Copy link
Author

vlsi commented Nov 29, 2022

mybatis products won't depend on the new module

Why so?

there already are many repos to maintain

WDYT of adding a module right in mybatis-3 repo? Of course, adding a new repo would be a maintenance burden.

I am not sure how this works, but if you could take care of it, I am OK, I think.

I've raised classgraph/classgraph#735

@harawata
Copy link
Member

I should have been clearer about my intent.
I basically want to maintain two TypeParameterResolvers independently.

I would expect future enhancements in the new module version of TypeParameterResolver (e.g. support for more complex usages) and that's more than welcome, but I do not plan to make such enhancement in the TypeParameterResolve of this repo, at least for now (as I wrote, I want to keep the internal version as simple as possible for better performance and easier maintenance).
Some changes might be ported between two versions if applicable.

Regarding the repo location, I just want to avoid burdening the team with incoming enhancement requests that are irrelevant to mybatis products.
May I ask why you want it to be maintained under mybatis org?
Is it about the groupId? (it would look more legit than my personal one, for sure)
If you think it's important, I'll consult with the team about how to proceed.

I have subscribed to the linked issue. Thanks!

@vlsi
Copy link
Author

vlsi commented Nov 29, 2022

I basically want to maintain two TypeParameterResolvers independently.

Oh, I see. I just did not think you wanted to have several copies.

May I ask why you want it to be maintained under mybatis org?

a) I thought having an extra module in https://github.com/mybatis/mybatis-3 would be simple enough refactoring: move TypeParameterResolver and tests into a different folder
b) As of now, the class suits my needs, so I am not sure there will be "lots of feature requests"
c) Moving class to a different folder is easier than creating a brand new repo, configure CI, release, etc

I have no preferences regarding groupid, package name, etc.

PS It looks like classgraph is more like "scan classes" rather than "hight-level API" :-/

@harawata
Copy link
Member

It's actually easier for me to setup a new personal repo.
https://github.com/harawata/typeparameterresolver/

I just deployed 1.0.0-SNAPSHOT.

<dependency>
  <groupId>net.harawata.reflection</groupId>
  <artifactId>typeparameterresolver</artifactId>
  <version>1.0.0-SNAPSHOT</version>
</dependency>

Please file an issue there for further discussion.

I plan to deploy 1.0.0 to the central and write the README this weekend.
In case your classgraph proposal is accepted, I can just ditch the repo. :)

@vlsi
Copy link
Author

vlsi commented Nov 30, 2022

For reference, the library has been released at https://github.com/harawata/typeparameterresolver

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