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

Include picocli-spring-boot-starter module in picocli project? #7

Open
remkop opened this issue Dec 5, 2018 · 14 comments
Open

Include picocli-spring-boot-starter module in picocli project? #7

remkop opened this issue Dec 5, 2018 · 14 comments

Comments

@remkop
Copy link

remkop commented Dec 5, 2018

Would you be interested in helping with a Spring Boot 2.0-compatible picocli-spring-boot-starter module to include in the picocli project?

It would be great if we could get it included in the standard spring boot starters list. This would give more exposure to the Spring Boot starter and picocli itself, and would make it very easy for users to leverage Spring's dependency injection in command line applications.

Picocli 3.x now offers a programmatic API that should make integration much easier than it used to be, and I'm open to any ideas on how to improve things further to facilitate integration (like better support for exit status etc.).

@kakawait
Copy link
Owner

kakawait commented Dec 5, 2018

Sure for everything @remkop but I/we need to update the starter to latest picocli version.

@kakawait
Copy link
Owner

@remkop Last response from mobile device was not really detailed

Would you be interested in helping with a Spring Boot 2.0-compatible picocli-spring-boot-starter module to include in the picocli project?

You mean merging picocli-spring-boot-starter inside picocli project? If true, yes I've no problem. I think could have better user experience if everything is on same place :)

About Spring Boot 2.0-compatible I started a branch picocli-3.0.0 that from what I remember since to work with. But not heavily tested.

It would be great if we could get it included in the standard spring boot starters list. This would give more exposure to the Spring Boot starter and picocli itself, and would make it very easy for users to leverage Spring's dependency injection in command line applications.

Totally agree. Then if become really popular maybe be part of https://start.spring.io/

Picocli 3.x now offers a programmatic API that should make integration much easier than it used to be, and I'm open to any ideas on how to improve things further to facilitate integration (like better support for exit status etc.).

I have to checkout more about Picocli 3.x to determine if something missing :)

@remkop
Copy link
Author

remkop commented Dec 17, 2018

You mean merging picocli-spring-boot-starter inside picocli project? If true, yes I've no problem. I think could have better user experience if everything is on same place :)

Yes, that is what I have in mind.

I have to checkout more about Picocli 3.x to determine if something missing :)

The release notes may be useful to get a high-level overview of what has changed.

About Spring Boot 2.0-compatible I started a branch picocli-3.0.0 that from what I remember since to work with. But not heavily tested.

Awesome! No rush, I'm thinking this will be a great addition to picocli 4.0. Some ideas of what may go into 4.0 are here: https://github.com/remkop/picocli/milestone/40

@kakawait
Copy link
Owner

@remkop how do you want to proceed?

What I could propose:

  1. Focus on Spring boot 2.0 & Picocli 3.0.0 integration
  2. Then merge base code inside official repo
  3. Then add doc section for Spring boot starter on your doc

However at the end of the year I'll be out of computer & internet. I don't think I'll have time to finish 1. before 2019

@remkop
Copy link
Author

remkop commented Dec 18, 2018

@kakawait Sounds great! Very happy we can work on this together.

@remkop
Copy link
Author

remkop commented Dec 18, 2018

An alternative may be to fork the picocli project from the beginning, and do a pull request from your branch. Since this will be an additional module (or modules), we can merge as early and as often as necessary.

I am planning to do a picocli 3.9 release end of the week and then merge my changes for 4.0 into master. That may be a good time for you to create your fork.

@kakawait
Copy link
Owner

@remkop and what do you think about:

New starter that will support Spring boot 2, will depend on picocli 4 (or 3, need to determine) version?

That means no more support for older version?

@remkop
Copy link
Author

remkop commented Dec 18, 2018

I’m not planning to introduce breaking changes with picocli 4.0, so even if the new starter would pull in picocli 4.0 by default, it very likely will still work with picocli 3.x. However if there’s some incompatibility I’m fine with the new Spring Boot module requiring picocli 4.x. I also think it’s fine to require Spring Boot 2.

@remkop
Copy link
Author

remkop commented Dec 20, 2018

FYI, when I was contributing picocli support to Micronaut, I improved picocli's support for dependency injection.

All main entry points now accept a factory that is used every time picocli constructs an object (I remember that this factory was your idea, it is now implemented properly!)

This factory facilitates dependency injection; the manual has a section on this with an example Guice Injector.

This factory is also how Micronaut's PicocliRunner is implemented. I suspect (but don't know enough about Spring) that we can introduce a similar PicocliSpringRunner. Perhaps the PicocliAutoConfiguration can be simplified a lot (if it is still needed).

@remkop
Copy link
Author

remkop commented Dec 20, 2018

Not sure if this is relevant, but this blog post about Spring performance may contain some useful tips on how to keep our starter performant.

@remkop
Copy link
Author

remkop commented Feb 1, 2019

I merged my initial changes for 4.0 to master. Bit later than expected, sorry about that.

@kakawait
Copy link
Owner

kakawait commented Feb 3, 2019

@remkop btw someone from community @colinkuo start working on 3.9 I'll start from that point to merge his work #10

@remkop
Copy link
Author

remkop commented Apr 6, 2019

@kakawait, how are things?

I am interested in improving the Spring Boot support in picocli 4.0.

The current Spring Boot integration example in the picocli manual works okay for top-level commands, but looking at issue remkop/picocli#658, not for subcommands.

I think this is because we should provide an implementation of picocli's IFactory that takes beans from the Spring ApplicationContext instead of instantiating them directly. I believe this is part of what the starter provides.

Do you think you will be able to help merge your starter into picocli?

kakawait added a commit that referenced this issue Apr 21, 2019
See `ApplicationContextAwarePicocliFactory`

fixes #9 and remkop/picocli#658
related to #7

---

[x] Impl
[x] Test
[ ] Sample
[ ] Doc
kakawait added a commit that referenced this issue Apr 22, 2019
See `ApplicationContextAwarePicocliFactory`

fixes #9 and remkop/picocli#658
related to #7

---

- [x] Impl
- [x] Test
- [ ] Sample
- [x] Doc
kakawait added a commit that referenced this issue Apr 22, 2019
See `ApplicationContextAwarePicocliFactory`

fixes #9 and remkop/picocli#658
related to #7

---

- [x] Impl
- [x] Test
- [x] Sample
- [x] Doc
kakawait added a commit that referenced this issue Apr 22, 2019
See `ApplicationContextAwarePicocliFactory`

fixes #9 and remkop/picocli#658
related to #7

---

- [x] Impl
- [x] Test
- [x] Sample
- [x] Doc
@remkop
Copy link
Author

remkop commented May 24, 2019

@kakawait Hi! Quick update from my side: I've made good progress on the picocli annotation processor for Graal integration. I'm hoping to release picocli-4.0-beta-1 next week.

The remaining big ticket item for picocli 4.0 is Spring support.
I would like to include the following:

For the initial release, just the factory may be sufficient, as long as we provide documentation (example) for how users would use this in their application. We can add other features to make configuration easier/more automatic in later releases. (It's easier to add things than to take them out...) :-)

Do you think you will have time to work on this? If not, that is fine, I can take a stab at it and perhaps ask you to do a review... What do you think?

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