talsma-ict / context-propagation

Propagate snapshots of ThreadLocal values to another thread
Apache License 2.0
18 stars 8 forks source link

Cleanup deprecations to develop next major version (v2) #515

Closed sjoerdtalsma closed 2 weeks ago

sjoerdtalsma commented 3 weeks ago

Main tasks to create new development baseline for next major release (v2).

coveralls commented 3 weeks ago

Coverage Status

coverage: 94.949% (+3.3%) from 91.679% when pulling 74c436ac6991ef3f5a8e6f4c039e97af09d663a0 on develop-v2 into 72abd11226d196266225ce791588f44cf1260767 on develop.

sjoerdtalsma commented 2 weeks ago

@Marcono1234 I hate to ask, but since you contributed before, could you have a look at this pull request I have prepared as new baseline for a greatly simplified, quicker 2.0 version?

By no means you have to do a full, detailed review (although welcome). A quick opinion on the API changes would already be greatly appreciated! 🙏

sjoerdtalsma commented 1 week ago

Sorry for the delay. I am currently not using this library, so I don't know how the changes and the new API feels for users. However, to me the changes look good and reasonable because they really simplify multiple parts.

I did not have an in-depth look at the changes, but hopefully the review comments are useful nonetheless.

@Marcono1234 thank you so much for the review and the many issues you caught!

I am contemplating on creating actual modules instead of the automatic modulenames currently used. But those will definitely require tests with both a java8 and java 9+ test project, so still quite a bit of work, also because the serviceloader mechanism works differntly then.

Marcono1234 commented 1 week ago

Just in case you aren't aware of it, the ModiTect Maven Plugin can be used to add a module descriptor to the JAR. And if you specify <jvmVersion>9</jvmVersion>, then it will be a Multi-Release JAR, and the module-info.class should (in most cases) not cause issues for Java 8 users of your library.

But I am not sure if it is worth it adding a module descriptor instead of using automatic module names at the moment.