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

Command Line Runner API changes #381

Merged
merged 8 commits into from
Oct 16, 2024

Conversation

Hello-zoka
Copy link
Collaborator

@Hello-zoka Hello-zoka commented Oct 9, 2024

Description of changes made

CommandLineRunner wasn't returning the exit code of the execution, which made Errors/Warnings/stdout indistinguishable. This PR fixes it. Please, make sure to test it as well since it changes the core logic.

Closes #298 and #361

Release

We probably don't want to include it in upcoming major releases to make sure everything works smoothly.

Notes for merging

Conflicts with PR #376

  • I have checked that I am merging into the correct branch

@Hello-zoka Hello-zoka added bug Something isn't working Ready for review PR redy for review important labels Oct 9, 2024
@Hello-zoka Hello-zoka linked an issue Oct 9, 2024 that may be closed by this pull request
Copy link
Collaborator

@Vladislav0Art Vladislav0Art left a comment

Choose a reason for hiding this comment

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

I skimmed through the implementation (did not test it; did not look at every file). I will make a more through review tomorrow/after the release.

@Hello-zoka Hello-zoka added changes requested Reviewer(s) left some comments that should be addressed by PR maintainer and removed Ready for review PR redy for review labels Oct 9, 2024
@pderakhshanfar pderakhshanfar added Urgent Urgant PR and removed important labels Oct 11, 2024
@pderakhshanfar
Copy link
Collaborator

@Hello-zoka I agree with Vladislav's comments, please:
1- fix his comments
2- Resolve merging issues

Then, we will check the code again. Thanks

@Hello-zoka Hello-zoka added Ready for review PR redy for review and removed changes requested Reviewer(s) left some comments that should be addressed by PR maintainer labels Oct 14, 2024
Copy link
Collaborator

@Vladislav0Art Vladislav0Art left a comment

Choose a reason for hiding this comment

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

I left some recommendations and suggestions. Nothing critical, although I advise you to consider them.

👍

Iurii Zaitsev added 5 commits October 16, 2024 00:26
# Conflicts:
#	core/src/main/kotlin/org/jetbrains/research/testspark/core/test/java/JavaTestCompiler.kt
#	core/src/main/kotlin/org/jetbrains/research/testspark/core/test/kotlin/KotlinTestCompiler.kt
…command-line-runner

# Conflicts:
#	core/src/main/kotlin/org/jetbrains/research/testspark/core/test/TestCompiler.kt
#	core/src/main/kotlin/org/jetbrains/research/testspark/core/test/java/JavaTestCompiler.kt
#	core/src/main/kotlin/org/jetbrains/research/testspark/core/test/kotlin/KotlinTestCompiler.kt
#	src/main/kotlin/org/jetbrains/research/testspark/tools/TestProcessor.kt
@Hello-zoka
Copy link
Collaborator Author

Ready to merge, if there are no problems with @pderakhshanfar @arksap2002

@pderakhshanfar pderakhshanfar merged commit fe4c48c into development Oct 16, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Ready for review PR redy for review Urgent Urgant PR
Projects
None yet
3 participants