Skip to content

refactor: introduced jbang-core library - #1128

Draft
quintesse wants to merge 1 commit into
jbangdev:mainfrom
quintesse:lib
Draft

quintesse wants to merge 1 commit into
jbangdev:mainfrom
quintesse:lib

Conversation

@quintesse

Copy link
Copy Markdown
Contributor

This is just to start up the discussion on how to go about creating a jbang library that people could use to create plugins or tools or in any other way interact with Jbang.

Right now I simply hacked the project in two: where almost all cli code remains in the current top-level project and all the rest was moved to a new jbang-core sub project.

It doesn't currently compile because I'm not very well-versed with Gradle, but it's 90% there I'm sure.

Also, the code hasn't been restructured in any way which means we're actually "overlaying" packages (dev.jbang.cli exists in both projects for example) which might cause problems, especially when Java's module system gets more and more strict.

Anyway, doing it like this was about an hour's work, which means it won't be that hard to re-do this PR if/when we actually decide to go ahead with this.

@maxandersen

Copy link
Copy Markdown
Collaborator

it makes sense we go in this direction - i'm wondering if using something like doing the refactoring in place and use archunit to ensure we don't have any deps to cli in core packages (i.e. https://www.archunit.org/use-cases) and then do the module split after that ?

@quintesse

Copy link
Copy Markdown
Contributor Author

Never heard about archunit before, looks cool and could be useful indeed.

@maxandersen

Copy link
Copy Markdown
Collaborator

is time ripe for redo on this one or wdyt?

@quintesse

Copy link
Copy Markdown
Contributor Author

Sure, the idea is still valid.

Btw, what would be the names of the artifacts? Because we already have a jbang-cli in Maven. So would that mean extracting all non-cli related things into a jbang-core (or some other name) and make jbang-cli depend on that? And jbang-cli-all would still be the "uber jar"?

@maxandersen

Copy link
Copy Markdown
Collaborator

Any reason why jbang-cli wouldn't still be the fat jar?

@quintesse

Copy link
Copy Markdown
Contributor Author

Any reason why jbang-cli wouldn't still be the fat jar?

It isn't right now either, isn't it? I see a jbang-cli.jar with a size less than 1 MB and a jbang-cli-all.jar of multiple MBs, so I just assumed the latter was the uber jar. (I never really took a look at the Maven artifacts before, so I might be understanding things wrong)

@maxandersen

Copy link
Copy Markdown
Collaborator

Ahhh sorry misunderstood you. Thought you said there would be another module.

But no all good and yes. Jbang-cli would have a fat jar

@maxandersen

Copy link
Copy Markdown
Collaborator

After reviewing this PR against today's codebase, the good news is that the separation is much cleaner than it was 4.5 years ago. The only reverse dependency (core → cli) was just two things: ExitException and the EXIT_* constants in BaseCommand.

#2602 fixes that — moves ExitException to dev.jbang and extracts ExitCodes. After that lands, the non-cli packages have zero imports from dev.jbang.cli.

Options going forward

Approach Effort Benefit
#2602 only (fix package layering) ✅ Done Breaks the reverse dependency. Enables future split without blocking on it
Gradle multi-project split (what this PR proposed) Medium — build config, test split, CI Clean dependency enforcement, publishable jbang-core artifact
java-library + api vs implementation Low Enforce API boundaries without subprojects
Do nothing beyond #2602 Zero At 26K LoC, a multi-module build adds friction for marginal gain

Recommendation

Close this PR once #2602 merges. The architectural fix is the important part. The full Gradle split can be a fresh PR when there's an actual consumer of jbang-core (plugin SDK, IDE integration, etc.) — it'll be straightforward since the package boundaries are now clean.

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

2 participants