Closed metteo closed 2 years ago
Hi @metteo, we require contributors to sign our Contributor License Agreement, and we don't have yours on file. In order for us to review and merge your code, please access https://www.artima.com/cla/choose-type to sign our Contributor License Agreement. Your effort is highly appreciated. Thank you.
@cla-bot[bot] check
The cla-bot has been summoned, and re-checked this pull request!
@katrinsharp FYI
@cheeseng do you process PRs or should I ask someone else?
@metteo Er, I don't work on this repo much, perhaps @katrinsharp ?
@cheeseng thanks for the response. I already called @katrinsharp but she didn't respond. Maybe @bvenners could help?
@cheeseng thanks for the response. I already called @katrinsharp but she didn't respond. Maybe @bvenners could help?
Sorry, was sidetracked by some stuff. I'll review, merge and build this week.
No problem. I expected that you will need some time to process the PR. Just got a little worried that I didn't get any response mentioning it will be processed in foreseeable future 😁
Hi @metteo Thank you for submitting your PR. Does your PR need any updates to main README? Any build instructions that would be useful?
Release related plugins are in separate release
profile. If you use release plugin it is activated automatically, otherwise you need to use -Prelease
on the command line.
Using JDK newer than 8 might be tricky together with current maven compiler setup (source=8, target=8 instead of release=8). That's why github action I added is using JDK 8 for CI.
Should I update the README with above info?
@katrinsharp any updates?
@katrinsharp any updates?
My apologies, yes. Please resolve conflicts as I've merged another PR, and I'll publish a new version this weekend. Thank you!
@katrinsharp PR rebased.
@katrinsharp I saw you merged another PR so I rebased this one to resolve conflicts.
I would expect this gets merged next. If you are hesitant in merging, please let me know why so I can apply appropriate changes.
@metteo I'd like to go over your PR once again to make sure it is good-to-go and then will merge. Please stay tuned. Thank you.
@bvenners I went through the PR changes and tested run the integration test with:
> mvn clean integration-test
and when I go into target/it/redirect-output/target/scalatest-reports I can see the scalatest-output.txt which contains the following:
Discovery starting.
Discovery completed in 68 milliseconds.
Run starting. Expected test count is: 1
AppTest:
Our example App
- should run
Run completed in 108 milliseconds.
Total number of tests run: 1
Suites: completed 2, aborted 0
Tests: succeeded 1, failed 0, canceled 0, ignored 0, pending 0
All tests passed.
Seems working great, @bvenners please review and consider approving so I can go ahead and merge this.
Thanks!
@cheeseng please try mvn clean verify
instead. integration-test
phase executes failsafe
plugin but it doesn't verify if tests failed or succeeded (the results are in a file to allow post-integration-test
phase to complete). That's what the github action does anyways ;)
Hi
This change will make the build logs shorter for cases when the tests produce lots of logs. Please review.
Details:
Regards Greg