search

rmpestano / cukedoctor

BDD living documentation using Cucumber and Asciidoctor: https://goo.gl/Yp3NiU
https://rmpestano.github.io/cukedoctor/
Apache License 2.0
121 stars 23 forks source link

Project is still using JDK 8, Maven 3.3 #263

Closed rdlopes closed 2 years ago

rdlopes commented 2 years ago

Hi Raphaël, hope you're well, as I understand the structure of the code, it's still using JDK 8/11 and Maven 3.3.3.

I have noticed a few other things:

I am sure that we could benefit from refreshing the project a bit, up to a great bump in performance.

Would you allow such a refresh? There would be backward compatibilities issues of course, but older users could still use versions up to 3.7.0. Newer users would benefit from the bump-up, version 4.0.0 I guess (if you follow semantic versioning).

rdlopes commented 2 years ago

Oh wow, I've just seen your recent updates, you've been busy ^^ So Maven is updated, that's neat!

rmpestano commented 2 years ago

Hey @rdlopes, regarding modernizing the code base, you're absolutely right! This project started a while ago and I think it was just recently that we migrated to Java 8.

About the logger, I still think we can rely on the standard one, I know it's painful but it will cause less trouble to the users of cukector, unless you have a use case where a different logger would be needed.

Would you allow such a refresh?

Definitely, let's go for it. We have quite an extensive test suite which will help us in any refactoring. I'd happily review and approve the PRs ;)

There would be backward compatibilities issues of course, but older users could still use versions up to 3.7.0.

I'm releasing 3.8.0 now and if let's say we decide to move to stay in JDK 8 but make a lot of internal refactorings to make the code clean/lean but keep the same API then we can just release 3.9.0 but if for example, we move to Java 11 or break change current API I'd go for v 4.0.

@andrewesweet is helping a lot with the project and might also weigh in but I'm all in for modernizing the code, I think it's also a go way for new contributors like you to learn the codebase.

Cherers!

rdlopes commented 2 years ago

Glad to hear @rmpestano !

Hey @andrewesweet, nice to meet you, I'm Rui, new to contributing but I'm actually quite experienced with cukedoctor and cucumber, I've been using them for years now ^^

I actually have a repo with cukedoctor 3.7.0, cucumber 7 and junit5

I plan to:

Don't know up to where I'm going to fill my duty, but I feel the need to pay back to cukedoctor, this lib has been a great companion over the years !

Will push progress and PRs here.

rdlopes commented 2 years ago

BTW, do you have formatting rules ? If none, I would suggest google-java-format cause it's a universal standard, but some have strong opinions on formatting.

rmpestano commented 2 years ago

switch to Java 17, latest LTS for Java

I think we can agree on Java 11 for baseline as it's quite adopted, I think if we go for 17 we might block a lot of users from upgrading cukdoctor

BTW, do you have formatting rules ? I would suggest google-java-format cause it's a universal standard, but some have strong opinions on formatting.

We don't have any. Personally at work I always rely on IntelliJ defaults but I agree that we should define one for this project as we might have contributors using different ides.

I think google one is fine, I just suggest to do all the formating it in one separated PR.

rdlopes commented 2 years ago

OK, so formatting will go first, I activated the dynamic analyzes on my IntelliJ and it's red and yellow all over the place 😰 PR #265 has been created for that matter.

rdlopes commented 2 years ago

I think we can agree on Java 11 for baseline as it's quite adopted, I think if we go for 17 we might block a lot of users from upgrading cukedoctor

They have the option to stay on 3.8.0, I have doubts on not upgrading to latest LTS, Java 11 is from September 2018 and vulnerabilities are usually not an option for Java professionals.

Thing is, what's the use of upgrading if that's for falling into a deprecated version soon again?

rmpestano commented 2 years ago

Thing is, what's the use of upgrading if that's for falling into a deprecated version soon again?

Mainly because some developers rely on the infrastructure that their organization have and are not in control of which JDK their CI runs. I think we should upgrade once the JDK 17 adoption increases and I think that won't take too much as Spring is setting Java 17 as baseline for Spring 6 and Boot 3, that will speed up adoption I guess.

rdlopes commented 2 years ago

OK, so be it. I created #266 for fixing the remaining warnings/errors during build and build output, but I need to rebase first.

Noticed two things:

  1. there are obviously warnings and errors that I cannot remove since they are on purpose
  2. there is a cyclic dependency from cukedoctor-converter -> cukedoctor-maven-plugin when profile docs is active.

For 2. I guess the project cukedoctor-converter should be dependent on the last RELEASE version and not on the snapshot. Maven does not allow cyclic dependencies in tree graph :-(

rmpestano commented 2 years ago

For 2. I guess the project cukedoctor-converter should be dependent on the last RELEASE version and not on the snapshot. Maven does not allow cyclic dependencies in tree graph :-(

I think if we do that then cukedoctor maven plugin won't use latest converter when pushing to maven central, unless we release then separately

rmpestano commented 2 years ago

Nevermind, you're talking about cukectodor maven plugin used by the converter to generate cukedoctor docs...I think we can do what you suggested, yes.

rdlopes commented 2 years ago

Normally we need to separate between dependencies and usage.

But there's a nice thing coming up with your latest changes: We can use the LATEST keyword in Maven since you have updated the Maven version.

rdlopes commented 2 years ago

Sorry for the mess on last PR; created #267 for a clean base, formatting killed me!

rdlopes commented 2 years ago

Hi there, created #270 to update to JDK 11. This PR will be the second to last, before the update of language constructs in the code.

@rmpestano you were mentioning a Jenkins plugin to replace ? I should probably do this before updating source code with Java 11 constructs.

rmpestano commented 2 years ago

you were mentioning a Jenkins plugin to replace ? I should probably do this before updating source code with Java 11 constructs.

@rdlopes currently the plugin is discontinued because we had so many issues with Jenkins and AsciidoctorJ. One idea is to provide Antora integration (#178) so we have a simple way to publish the documentation generated by Cukedoctor.

To sumup we can move forward and upgrade to 11

rdlopes commented 2 years ago

Hi folks, here's the last PR for that issue #271 - removing a few code smells spotted by Sonar.