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

Support for Java language #367

Merged
merged 6 commits into from
Oct 13, 2023
Merged

Support for Java language #367

merged 6 commits into from
Oct 13, 2023

Conversation

kozusznik
Copy link
Contributor

Docker image for Java evaluator.

Docker image for Java evaluator.
@Kobzol
Copy link
Collaborator

Kobzol commented Oct 12, 2023

Hi, thanks, this looks good! Do you perhaps have some Java test project with the intended layout (e.g. a set of Java files, Maven configuration file, etc.), so that I could test it locally?

@kozusznik
Copy link
Contributor Author

Hi, thanks, this looks good! Do you perhaps have some Java test project with the intended layout (e.g. a set of Java files, Maven configuration file, etc.), so that I could test it locally?

Hi, thank you. You can use this example.

Copy link
Collaborator

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Apart from the mentioned nits, I have also pushed two changes that we need to make to properly support the Java pipeline.

I haven't tested tests, but build of the linked example project looks fine. The build log is quite… verbose, so it might be nice to hide it by default in a <details> HTML tag, like is being done for Rust (like this). But it's just a nice-to-have thing.

evaluator/images/java/Dockerfile Outdated Show resolved Hide resolved
evaluator/images/java/entry.py Outdated Show resolved Hide resolved
evaluator/images/java/entry.py Outdated Show resolved Hide resolved
evaluator/images/java/entry.py Outdated Show resolved Hide resolved
@kozusznik kozusznik force-pushed the master branch 3 times, most recently from 221307f to a0a0900 Compare October 13, 2023 10:23
This commit address comments from the pull request:
-replace some packages installed in docker image (in Dockerfile)
-fix errors and remove unused variables from the file entry.py
@kozusznik
Copy link
Contributor Author

Thank you again for the review. I addresed all comments.

@kozusznik kozusznik reopened this Oct 13, 2023
@kozusznik kozusznik requested a review from Kobzol October 13, 2023 10:32
Copy link
Collaborator

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Thanks! I pushed one additional fix, since the code should have newlines and not <br />, since it's displayed in <pre>.

@Kobzol Kobzol merged commit cac2fd2 into mrlvsb:master Oct 13, 2023
2 checks passed
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