bramp / ffmpeg-cli-wrapper

Java wrapper around the FFmpeg command line tool
BSD 2-Clause "Simplified" License
1.72k stars 413 forks source link

JDK Upgrade plans #269

Closed Euklios closed 8 months ago

Euklios commented 2 years ago

The question While working on #268, I noticed some TODOs regarding Immutable responses. I already have a second fork where I've replaced most pojos with Immutable responses, but that fork relies on records (JDK 16). So my question: Is there any plan on upgrading to newer JDK versions?

Benefits:

Additionally, JDK 8 already left oracles "Premier Support" and is only available in "Extended Support". Consider upgrading to a newer LTS release (11 or 17).

TODO:

Current JDK version: 16

Breaking changes:

Euklios commented 2 years ago

@bramp: Either way is fine, but if you are fine with upgrading, I would create a pull request for a newer Java version. It shouldn't entail more than setting a newer version; I'm currently running this library with JDK18. But I'd need the information before I can take some other tasks

bramp commented 2 years ago

Hey @Euklios, thanks for all your recent contributions! As you can most likely tell, I've been running this project in maintenance mode at best, and I've personally not used this in a production system in ~8 years.

So I would be happy for you to bump this to later versions, however, I'm just concerned if this will break anyone. So maybe if we bump up to JDK18, we bump the major version of the library.

Euklios commented 2 years ago

That's more or less in line with what I expected. I can relate completely; there are other priorities in life, especially if you don't need the library yourself. But it's an extremely useful tool and I depend a lot on it within my projects, so I wanted to share some things I patched for myself with the community. Thank's allot for the work!

Back to the topic at hand: There are currently more or less two approaches I'd see as feasible:

  1. Upgrading to the latest LTS and jumping from LTS to LTS afterward
  2. Upgrading to the latest release and jumping for consequetive releases

For libraries, I would argue jumping from LTS to LTS simply because some companies don't want to (or can't) upgrade to every intermediate release. So basically by not upgrading every half a year we could be a bit more inclusive, with the cost of waiting for new language features for up to a year.

I agree with the major bump, but I'd like to have more time for that one. I'd like to implement Immutable responses first, as they will probably result in some API incompatibilities (method instead of field or things like that), so basically perfect for a major release.

I will, for now, bump it to 17; if you want higher, that would be fine by me as well.

bramp commented 2 years ago

Aligning with LTS sounds great..I have to admit I don't use Java in my day to day anymore, so I'm out of the loop of best practices, etc.

I'm in no rush to bump, so if you want to figure out immutable objects as well, that sgtm.

For the jdk bump, let's try an ensure the travis continues to work. If we want to align with LTS, let's document that in the readme.

I'll find time today to review your other PRs, in hopes not to slow you down too much.

gillbates commented 1 year ago

It would be much better if we could merge with JDK 17 ...

Euklios commented 1 year ago

@gillbates: Sorry for the delayed reply. I'm currently on vacation, and I often don't have a network connection when I work on this.

I'm currently doing the upgrade one version at a time. Therefore, there will be a version using JDK 17. There need to be more compelling features in higher versions, so I'm likely going no further than that LTS.

I don't know how this will play out in future versions. But if interest exists, I might leverage features like Multi-Release Jar Files. But currently, I'm not even on that version, so not a problem right now.

Euklios commented 1 year ago

@bramp

While working on this, I stumbled across the FFmpegOutputBuilder#buildOptions function. It's supposed to serialize builder options but is out of sync with the actual class (according to the documentation). It's irrelevant within this project, and as it is outdated, I hope it's not enjoying too much usage outside.

Should we eliminate or start embracing it and implement corresponding tests?

bramp commented 1 year ago

With every year I forget more and more of this. Right now, I've pushed changes to make this work with Java 8, and I'll be pushing a new build shortly. If we wish to go beyond that, that's cool I just don't understand the impact, and at that point, maybe I just hand responsibility over to Euklios or other willing maintainers.

Euklios commented 8 months ago

With every year I forget more and more of this. Right now, I've pushed changes to make this work with Java 8, and I'll be pushing a new build shortly. If we wish to go beyond that, that's cool I just don't understand the impact, and at that point, maybe I just hand responsibility over to Euklios or other willing maintainers.

Sorry, I didn't notice this up until now. I'd be willing to maintain this project if you want to hand over some responsibility. But please, could I get permission to close issues? There are pages of answered questions that just clutter my view. I'm about to open an issue collection of other Issues to close.

Euklios commented 8 months ago

I'm closing this issue in favour of #316