search

kokorin / Jaffree

______ Stop the War in Ukraine! _______ Java ffmpeg and ffprobe command-line wrapper
Apache License 2.0
475 stars 82 forks source link

Added Support for Duration Objects. #417

Open Speiger opened 3 weeks ago

Speiger commented 3 weeks ago

Simple support for Duration Objects.

Since they are cleaner to work with than milliseconds directly or using weird number * timeunit multiplications. Not saying they are bad, but Duration objects are just cleaner to work with due to their helper functions.

I included tests and ran them with no errors. Though the drawTextWithSpecialCharacters tests fails on both PRs i made so far.

Speiger commented 3 weeks ago

Added tests for them to assure the logic works the exact same. Ran the tests locally and they work just fine.

From my side the PR is finished.

kokorin commented 3 weeks ago

From my side the PR is finished.

Please, pay attention to code style accepted in the project.

Speiger commented 3 weeks ago

@kokorin yep already did. If you need something extremely specific then please name them all in order :)

Speiger commented 3 weeks ago

@kokorin Let's make this simpler.

I think the code quality filter you have setup is extremely picky. (Especially in the documentation like WTF) Too picky IMO.

I went out of my way to make these patches and provided them in a reasonable state.

Anything extra is for you to uphold. In other words please to the final polishing cut yourself.

If you don't want to do that and think that mentality is unreasonable, then please close both prs. Because that just wastes time for everyone involved.

Speiger commented 2 weeks ago

@kokorin please provide a choice.

kokorin commented 2 weeks ago

From my side the PR is finished.

Please, pay attention to code style accepted in the project.

Already did

Speiger commented 2 weeks ago

Yeah, and at this point you lost me. I rather deal with a self maintained fork then with these, IMO excessive, documentation requirements. If you want to be picky that's fine, but also expect people not to 100% agree on that and ask you to meet halfway. I mean you had enough time to review the PR too.

Anyways, I work with my fork instead of helping.

Ignoring this issue. Have a nice weekend!

Speiger commented 2 weeks ago

Small fun fact, @kokorin Eclipse, my choice of IDE doesn't support Editor configs so in other words: I can't apply your style configs at all.

In other words unless I am switching the IDE, which i am not willing to, that style check won't be completed.

So yeah, the question comes again: I would like to see a response from you based on if you want to finish it or not.