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

227 dependency checker #233

Merged
merged 4 commits into from
Mar 3, 2022
Merged

227 dependency checker #233

merged 4 commits into from
Mar 3, 2022

Conversation

pihme
Copy link
Contributor

@pihme pihme commented Mar 3, 2022

Description

  • adds dependency checker
  • fixes dependency errors

Related issues

closes #227

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

  • The changes are backwards compatibility with previous versions
  • If it fixes a bug then PRs are created to backport the fix

Testing:

  • There are unit/integration tests that verify all acceptance criterias of the issue
  • New tests are written to ensure backwards compatibility with further versions
  • The behavior is tested manually

Documentation:

  • Javadoc has been written
  • The documentation is updated

@pihme
Copy link
Contributor Author

pihme commented Mar 3, 2022

@remcowesterhoud If you have some time, you could maybe take a look at this. Now the dependency checker is happy, but the tests are not running. There are two issues that I am aware of:

  • the dependency checker removed all the log simple dependencies saying they were not needed. But I think you added them on purpose
  • The testcontainers don't run now. I think this either has to do with the scope they were defined in, or maybe I need to add ignore statements to dependency checker.

I will take a second look after lunch. If you have any insights, please share.

@github-actions
Copy link

github-actions bot commented Mar 3, 2022

Unit Test Results

128 files  128 suites   6m 11s ⏱️
325 tests 325 ✔️ 0 💤 0
788 runs  788 ✔️ 0 💤 0

Results for commit 60e3217.

♻️ This comment has been updated with latest results.

@pihme
Copy link
Contributor Author

pihme commented Mar 3, 2022

@remcowesterhoud Also why are the tests failing but the test results are all green?

@remcowesterhoud
Copy link
Contributor

the dependency checker removed all the log simple dependencies saying they were not needed. But I think you added them on purpose

These are needed. They are not used directly in the code so that's probably why the dependency checker removes them. But we need to provide an slf4j implementation in our tests, otherwise we won't get any logging.

The testcontainers don't run now. I think this either has to do with the scope they were defined in, or maybe I need to add ignore statements to dependency checker.

It seems the dependency checker removed the java.annotation-api dependency. Again this is a dependency that is not used directly in our code. However, it is used in the code that is generated by gRPC, which is why there are compile errors.

Also why are the tests failing but the test results are all green?

Not quite sure what you mean, everything seems red to me?

@pihme
Copy link
Contributor Author

pihme commented Mar 3, 2022

Not quite sure what you mean, everything seems red to me?

#233 (comment)

@remcowesterhoud
Copy link
Contributor

Hmm that's interesting. I think it's because the pipeline failed on compilation. Therefore no new artifacts with test reports are published. I guess that on publishing it will download the latest artifacts for this PR, which would be the ones of the previous commit 🤔

@pihme pihme force-pushed the 227-dependency-checker branch from ff324fb to 5024e4d Compare March 3, 2022 12:28
@pihme pihme marked this pull request as ready for review March 3, 2022 13:18
@pihme pihme requested a review from remcowesterhoud March 3, 2022 13:18
Copy link
Contributor

@remcowesterhoud remcowesterhoud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one small question/suggestion 👍

extension/pom.xml Outdated Show resolved Hide resolved
@pihme pihme merged commit b045942 into main Mar 3, 2022
@remcowesterhoud remcowesterhoud deleted the 227-dependency-checker branch March 7, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add maven dependency checker plugin
2 participants