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

[Feature Request] Kotlin DSL #2450

Closed
AllanWang opened this issue Jun 13, 2019 · 15 comments
Closed

[Feature Request] Kotlin DSL #2450

AllanWang opened this issue Jun 13, 2019 · 15 comments

Comments

@AllanWang
Copy link
Contributor

Not tied to any version, but posted as of 7.x.x

Now that most of the libraries are kotlin first, will there be plans to introduce kotlin dsl?
Material Drawer has the benefit of other libraries such as https://github.com/zsmb13/MaterialDrawerKt, which makes writing code in kotlin much easier.

I think it would be worthwhile to have the two projects merged if both you and @zsmb13 approve; I don't think there will be much of a difference if you rewrote the DSL, since most of them retain the same names.

@mikepenz
Copy link
Owner

@AllanWang oh that is actually something we were thinking about, just had no time yet.

I like the idea though to work together with @zsmb13 as this project is now fully kotlin, so integrating the dsl would absolutely make sense.

@zsmb13 please let me know if it would be fine to integrate your great DSL?

@mikepenz mikepenz self-assigned this Jun 13, 2019
@zsmb13
Copy link
Contributor

zsmb13 commented Jun 13, 2019

This is an interesting proposition to be sure.

I am still actively maintaining the DSL library. There wasn't much to do in terms of keeping up with the base library for a while now, but I've started already looking into updating things for the 7.0.0 release.

I'd of course be thrilled if my library was made "official" in some manner (if there was an organization for these libraries, it could be moved there, but there isn't) and would see more users as well as possibly contributors.

I'm not sure if it should be bundled with the base library, even if that has been refactored to Kotlin. It provides an entirely new interface for using the library. It also doesn't have to be bundled together to function, as it just relies on extensions. I think it would make a lot of sense to still have this as a separate artifact that's optional for users, especially if someone is using the base library from a Java code base. They wouldn't want to pull in all that extra functionality that they'll never use.

Then, of course, there is the issue of naming. MaterialDrawerKt doesn't make much sense anymore if the base library is Kotlin as well (if only I anticipated this happening, but oh well), should be MaterialDrawer-DSL or something like that instead. Renaming is a significant endeavour that I wouldn't take lightly. Especially if the artifact gets renamed, I worry that many people would be cut off from updates that they'd otherwise be getting - there's no good way to notify all the users of the changes.

TL;DR:

  • I think the DSL should be bundled separately still, because the core library doesn't need to include that entirely different API. Especially Java users shouldn't need to pull in all that code.
  • The name of the DSL library doesn't make sense anymore, which is sad, but also seemingly very hard to fix.

I'm open to any and all thoughts & opinions on this, should be an exciting discussion!

@mikepenz
Copy link
Owner

@zsmb13 it is really great to see that you are still maintaining the library.

  • Based on the current details I was thinking of adding the DSL as a new module to the project. and it being a new artifact beside the core library providing the DSL.
  • Its source code would life directly beside the core in this repository giving it full visibility for people using the MaterialDrawer.
  • I'd love to add you as contributor to this repository so you can still directly maintain, and perhaps go with pull requests or whatever fits best.

As for you being afraid of people using your lib being cut off. I would assume as the MaterialDrawer is the sub dependency of your project people regularly check this repository too as it contains the core library.

@AllanWang
Copy link
Contributor Author

If package renaming is inevitable, I think the best bet would just be to deprecate everything with a warning pointing to the new source? Updating would be a matter of changing the imports if the dsl doesn't change.

I think having an extra module for extensions would be great, given official support and better visibility.
The problem becomes a change in ownership, which I'm not in the position to comment on.

The other main dependencies like FastAdapter and Iconics may also be in need of dsl (or at least some kotlin improvements), but there isn't really a good organization to bundle it in, beyond all of them being lead by Mike Penz

@zsmb13
Copy link
Contributor

zsmb13 commented Jun 20, 2019

Its source code would life directly beside the core in this repository giving it full visibility for people using the MaterialDrawer.

This makes sense, it could live in this repo as a separate module, and therefore a separate artifact 👍

I'd love to add you as contributor to this repository so you can still directly maintain, and perhaps go with pull requests or whatever fits best.

I'd certainly like to keep being the primary maintainer of this module, and contribute to it as directly possible. This is my largest claim to fame in the open source community so far, so it's a very, very important project to me. I assume you'd want to change its group ID to match the base library's, instead of being co.zsmb? How about the package name? It would be nice if either of those could be kept, but I think I'm also willing to give them up if necessary, and just be content that my work reaches more people.

I would assume as the MaterialDrawer is the sub dependency of your project people regularly check this repository too as it contains the core library.

I'm not sure people are checking even my repository's page. I can image a lot of people adding the dependency once and then just updating it based on IDE hints.

If package renaming is inevitable, I think the best bet would just be to deprecate everything with a warning pointing to the new source? Updating would be a matter of changing the imports if the dsl doesn't change.

I guess this would be the best possible approach, I unfortunately don't know of any way of redirecting people after changing coordinates. So I could release a fake version that puts up lots of deprecation messages, and perhaps has some -DEPRECATED postfix in the version name as well.


Other than the coordinate migration, one more technical consideration is that I've moved to publishing my library to MavenCentral, and I would very much prefer it to continue to be available from that repository, if possible - I've even written an article at one point about issues that I've personally faced using jcenter. You couldn't publish it under my group ID of course, because I've been using my own GPG keys to sign the artifacts, but if the group ID was changed, it would be possible.

This is a non-issue if you wish to stick to just publishing all these libraries from jcenter, but I really would recommend the MavenCentral approach (though it's admittedly very tedious), and would be happier if my work was distributed through there.

@mikepenz
Copy link
Owner

I will give a long answer to the first topics later.

Related to your comments for MavenCentral. I absolutely agree. I still publish my libraries to maven central too, and I still sign my libraries via a GPG key to ensure their integrity. Because in my opinion it is very important to have this way of verification that the artifact was published from a trustworthy source.
I just added the option via jCenter as it gives a faster and more direct way of giving updates. maven central tends to take quite some time to update sometimes. Also the sync form mavenCentral to jCenter sometimes caused problems for people. so I just decided I will publish to both. and have the artifacts on jcenter with proper description and stuff. :)

@zsmb13
Copy link
Contributor

zsmb13 commented Jun 20, 2019

Ah, okay, that's great news then. I never knew I could pull the base library from MavenCentral as well!

@mikepenz
Copy link
Owner

So in general I would think it would be best to have the artifact in the same group hosted, as it is common (at least I do that regularly) to find artifacts from the same group to see if a lib has other modules or so. Also it makes it more easier for people to understand that it works together.
Related package name. In general I'd prefer to have it the same as the main lib, just to make it understandable for people what they actually import in their source files.

But based on the reach of your library and the fact that many people already use it, I guess for the sake of simplicity for the upgrade we could keep the package name for the existing classes and slowly migrate towards the main libraries package name with new classes.
One thing which we should definitely do is to have a proper license header in the source files of your DSL with your name so that this visibility is guaranteed. Also adding credits for it directly in the README

Related the deprecation phase. My feeling would be that it would be enough to have a notice in the repo and link people over and do an update with a deprecation on the main class to note that things were moved and that the dependency is now maintained somewhere else.
There is somehow a possibility on maven central to specify that a library was moved to a different group or so, but I have no clue how to do this :D

@mikepenz
Copy link
Owner

mikepenz commented Jul 6, 2019

(just a heads up)
I guess this discussion might still be ongoing a bit longer, so most likely I will go ahead and release v7.0.0 without including a DSL yet.

@zsmb13
Copy link
Contributor

zsmb13 commented Jul 6, 2019

I think we've figured out most of the merging stuff, but I still have updating to do on the DSL for the new base library version. If you want to release 7.0.0 soon, you should do that on its own, because I'll be a bit behind with catching up with the DSL.

When I'm done with that, I think we can quickly hash out the details of how to move it into this repo. There don't seem to be any blocking technical issues left, we'll just have to work out some minor details, probably over chat or something.

@mikepenz
Copy link
Owner

mikepenz commented Jul 6, 2019

@zsmb13 sounds good to me.

btw. I had another idea which might be a potential variant we could go.

You keep your repo, package name. but we adjust the module so it follows the module structure needed for releasing it as part of the main library. then include it as git submodule into the main repo. after that we then publish it under the same maven package. and have the main README properly document and inform users.

But all this we can discuss in a chat as you proposed.

@AllanWang
Copy link
Contributor Author

Any updates? With the recent breaking changes due to other library updates, the dsl can't be used until it too has a version bump

@mikepenz
Copy link
Owner

mikepenz commented Dec 8, 2019

Funny story. @zsmb13 and me actually met at the kotlinconf this week 😂

@mikepenz
Copy link
Owner

mikepenz commented Jul 5, 2020

Closing this for inactivity. Additionally v8 is using a new API which is more kotlin friendly and should basically make the kotlin DSL obsolete. :)

Open for improvements.

Thanks so much for the great discussion in here.

Also on that note. Congratulations @zsmb13 for getting to be a GDE :D

@mikepenz mikepenz closed this as completed Jul 5, 2020
@zsmb13
Copy link
Contributor

zsmb13 commented Jul 5, 2020

Agreed, I actually haven't updated the Kt wrapper library past 7.x, because I don't see how it could do a much better job than the regular library. Plus now it couldn't be super seamless, as you'd need to do the XML setup parts of using the library anyway.

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

No branches or pull requests

3 participants