beanshell / beanshell

Beanshell scripting language
Apache License 2.0
815 stars 183 forks source link

Set up Jitpack build #722

Open TarCV opened 1 year ago

TarCV commented 1 year ago

Fixes #715

Hi, This PR setups a multi-module build in Jitpack, so that beanshell should be accessible at com.github.beanshell.beanshell:bsh and bsh-bsf-engine module at com.github.beanshell.beanshell:bsh-bsf-engine. Hope, it would be useful.

I've also made some additional changes that were required for successful builds or seemed logical to me:

Feel free to correct me if you think something is wrong, or, maybe, there are some changes that should be split into a different PR.

nickl- commented 1 year ago

Fixes #715

Hi, This PR setups a multi-module build in Jitpack, so that beanshell should be accessible at com.github.beanshell.beanshell:bsh and bsh-bsf-engine module at com.github.beanshell.beanshell:bsh-bsf-engine. Hope, it would be useful.

Awesome!! This will help a lot. Ta!!

I've also made some additional changes that were required for successful builds or seemed logical to me:

* Reverted checkstyle version to the version before update by dependabot, as the new version is not compatible with current maven checkstyle plugin version. Another way to fix that would be to update maven plugin according to  https://maven.apache.org/plugins/maven-checkstyle-plugin/history.html

* checkstyle found some issues in bsh-bsf-engine, so I fixed them

* changed bsh-bsf-engine group and version - I suppose they should match bsh module?

* updated bsh dependency in bsh-bsf-engine - again, I think it should match latest bsh?

The main project is using checkstyle 9.2.1 and has no problems.

https://github.com/beanshell/beanshell/blob/b1998b13eca9e945cd312f327d5b84cf0a9fa1fe/pom.xml#L199-L209

But we should exclude bsh-bsf-engine from the jitpack build, in fact it should actually be migrated to a separate project.

Feel free to correct me if you think something is wrong, or, maybe, there are some changes that should be split into a different PR.

I think it would be better to split the two, lets do the jitpack in one PR, and then deal with bsh-bsf-engine in a separate PR, that will be mach easier.

TarCV commented 1 year ago
* Reverted checkstyle version to the version before update by dependabot, as the new version is not compatible with current maven checkstyle plugin version. Another way to fix that would be to update maven plugin according to  https://maven.apache.org/plugins/maven-checkstyle-plugin/history.html

The main project is using checkstyle 9.2.1 and has no problems.

Yes, but the main project is using different versions - plugin 3.2.1 and checkstyle 9.2.1. That's closer to that compatibility table - plugin 3.2.1 should be compatible with checkstyle 9.3. I suppose 9.3 API is quite close to 9.2.1, thus no problems. bsh-bsf-engine is using maven plugin 2.17 which should be compatible with 6.11.2. And checkstyle 8.29 is missing a method required by that plugin version and which was removed at sometime after 8.11.

Probably it makes sense to update bsh-bsf-engine to use plugin 3.2.1 and checkstyle 9.2.1 (or even 9.3)?

But we should exclude bsh-bsf-engine from the jitpack build, in fact it should actually be migrated to a separate project.

I think it would be better to split the two, lets do the jitpack in one PR, and then deal with bsh-bsf-engine in a separate PR, that will be mach easier.

Ok, that makes sense. BTW, as bsh-bsf-engine will not be a part of the build, we can actually publish just a single module with coordinates com.github.beanshell:beanshell. Would it be better to implement that or to keep com.github.beanshell.beanshell:bsh?

nickl- commented 1 year ago

Probably it makes sense to update bsh-bsf-engine to use plugin 3.2.1 and checkstyle 9.2.1 (or even 9.3)?

Both being the same makes sense, and upgrading plugins to their latest versions also makes sense.

BTW, as bsh-bsf-engine will not be a part of the build, we can actually publish just a single module with coordinates com.github.beanshell:beanshell. Would it be better to implement that or to keep com.github.beanshell.beanshell:bsh?

I still know too little about jitpack to be able to comment on this. Where can we get documentation?

The former looks simpler com.github.beanshell:beanshell but we historically build the packages as bsh so is there any way around this?

TarCV commented 1 year ago

Sorry for late answer.

BTW, as bsh-bsf-engine will not be a part of the build, we can actually publish just a single module with coordinates com.github.beanshell:beanshell. Would it be better to implement that or to keep com.github.beanshell.beanshell:bsh?

I still know too little about jitpack to be able to comment on this. Where can we get documentation?

The former looks simpler com.github.beanshell:beanshell but we historically build the packages as bsh so is there any way around this?

The documentation is at https://docs.jitpack.io/building/ From my understanding Jitpack allows only two naming variants for GitHub projects: com.github.<organisation>:<repository> for single module projects and com.github.<organisation>.<repository>:<module-name> for multi-module projects. Also in the second case com.github.<organisation>:<repository> is still created, but it depends on all published modules of a multi-module project. And they support custom domains as groups: https://docs.jitpack.io/faq/ > Can I use my own domain name?

So I think the only way to use bsh name without renaming this repository is to use multi-module build (perhaps with only one module - bsh). That would give either com.github.beanshell.beanshell:bsh coordinate or, for a custom domain, <custom-domain>:bsh (e.g. org.apache-extras:bsh for apache-extras.org domain).

nickl- commented 1 year ago

Sorry for late answer.

No sweat! Been preoccupied myself...

So I think the only way to use bsh name without renaming this repository is to use multi-module build (perhaps with only one module - bsh). That would give either com.github.beanshell.beanshell:bsh coordinate or, for a custom domain, :bsh (e.g. org.apache-extras:bsh for apache-extras.org domain).

That's a bummer! Will look into it some more, thank you!