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

Add test code from java_common #1

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Add test code from java_common #1

merged 1 commit into from
Apr 19, 2024

Conversation

MrCreosote
Copy link
Member

When packing java_common from jitpack.io, only the code in main/java is packed, (which makes sense) which doesn't include any of the code here. Moving the test halper code to main/java would mean that junit and hamcrest would now be in the runtime classpath, which we don't want, so it seems the least bad option is to make a new library.

Also copied over the blobstore and minio controllers from workspace. The mysql controller is unused to my knowledge so it was not copied.

When packing java_common from jitpack.io, only the code in main/java is
packed, (which makes sense) which doesn't include any of the code here.
Moving the test halper code to main/java would mean that junit and
hamcrest would now be in the runtime classpath, which we don't want, so
it seems the least bad option is to make a new library.

Also copied over the blobstore and minio controllers from workspace.
The mysql controller is unused to my knowledge so it was not copied.
@MrCreosote MrCreosote requested a review from Xiangs18 April 14, 2024 00:25
Copy link

codecov bot commented Apr 14, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@Xiangs18
Copy link

In blobstore repo, there were also a bunch of controllers: https://github.com/kbase/blobstore/tree/develop/test.
Do they serve different purposes?

@Xiangs18
Copy link

Also, I am not sure why we need @SuppressWarnings("unused") in all noop(). Because foo is unused?
I ran gradle build with and without @SuppressWarnings("unused"), I did not see any difference. Am I missing something?

@MrCreosote
Copy link
Member Author

MrCreosote commented Apr 19, 2024

In blobstore repo, there were also a bunch of controllers

  • Mongo is in this repo
  • Minio is in the workspace repo because that's the only java service that uses it. If more services start using it we should move it here Minio is in this repo
  • Auth lives in the auth repo

@MrCreosote
Copy link
Member Author

MrCreosote commented Apr 19, 2024

Also, I am not sure why we need @SuppressWarnings("unused")

It prevents IDEs from complaining about unused variables. There's also probably some way to get Gradle to report out on compile warnings, but I guess by default it doesn't

Copy link

@Xiangs18 Xiangs18 left a comment

Choose a reason for hiding this comment

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

LGTM!

@MrCreosote MrCreosote merged commit 0ed4cbe into main Apr 19, 2024
4 checks passed
@MrCreosote MrCreosote deleted the dev-add_code branch April 19, 2024 20:50
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.

2 participants