Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review dependencies added for convergence #8967

Closed
npepinpe opened this issue Mar 23, 2022 · 0 comments · Fixed by #9259
Closed

Review dependencies added for convergence #8967

npepinpe opened this issue Mar 23, 2022 · 0 comments · Fixed by #9259
Assignees
Labels
area/project Marks an issue as related to project management (e.g. PR templates, editor config, etc.) kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. version:8.1.0-alpha2 version:8.1.0 Marks an issue as being completely or in parts released in 8.1.0

Comments

@npepinpe
Copy link
Member

Description

This issue is about going over our long list of dependencies defined in the dependency management section, and removing those which were added for dependency convergence but aren't required anymore.

@npepinpe npepinpe added the kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. label Mar 23, 2022
@npepinpe npepinpe added the area/project Marks an issue as related to project management (e.g. PR templates, editor config, etc.) label Apr 20, 2022
@npepinpe npepinpe self-assigned this Apr 25, 2022
zeebe-bors-camunda bot added a commit that referenced this issue May 2, 2022
9259: Cleanup of dependencies and POMs r=npepinpe a=npepinpe

## Description

Primarily cleans up the parent module POM, removing dependencies that were added for convergence but are not required anymore. Dependencies added for convergence are now put in their own section to make this task easier in the future, along with comments on which libraries had conflicts.

Additionally, for some POMs which were modified due to the above tasks, I've separated production and test dependencies. Again this is to make it easier to perform cleanup and maintenance, but it also helps us more quickly understand what exactly will be included in the final JAR at a glance.

Modules which generated SBE code were slightly fixed - we can look at the Wiki page for SBE Tool with Maven, which proposed a slightly different approach that doesn't require us to specify sbe-tool as a dependency. We can tell the exec plugin to use the plugin dependencies instead of the project's dependencies, and then sbe-tool is just a
dependency of that plugin.

Finally, this PR also removes usage of journal test-jar via the `LogCorrupter`. While the new
tests is potentially not as accurate, using the journal's log corrupter utility is essentially leaking implementation details of the journal to Raft. These tests should not care how the log is corrupted - how to corrupt it is implementation detail, in a way.

The tests should be refactored long term to be journal implementation agnostic, but as is, they're fine.

Additionally, we should not use test JARs in general. Not only does that import test classes, but most of the classes or utilities we write for one off tests are often not meant to be used by other tests - they're not documented, poorly tested, and since we don't expect them to be used else where, we're more likely to change the behavior.

Using test JARs in another module is not only further coupling the modules, but it's worse as it usually couples implementations together. Now not only are we coupling to the API of the module, but to its test utilities and most likely to its implementation details, because usually that utility was created to test the implementation of said module.

## Related issues

closes #8967 



Co-authored-by: Nicolas Pepin-Perreault <nicolas.pepin-perreault@camunda.com>
@Zelldon Zelldon added the version:8.1.0 Marks an issue as being completely or in parts released in 8.1.0 label Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/project Marks an issue as related to project management (e.g. PR templates, editor config, etc.) kind/toil Categorizes an issue or PR as general maintenance, i.e. cleanup, refactoring, etc. version:8.1.0-alpha2 version:8.1.0 Marks an issue as being completely or in parts released in 8.1.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants