-
Notifications
You must be signed in to change notification settings - Fork 187
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 IMAPReply tests and documentation fixes #165
Add IMAPReply tests and documentation fixes #165
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me. The GH Actions failure has a test failure but that's already happening on master
. Thanks @jkbkupczyk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question here, are those comments (kate: ident...
) necessary?
Let's fix master before we do anything else. I'll look today. It is ok to add the Junit test dependency for parameterized testing. |
OK, I finally nailed the bulletproofing on the failing test. Would you please rebase this PR on master? Then we should see a green build... TY! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkbkupczyk Please rebase on master.
Ah, no, got another random failure on master for Java 8 on Windows, which is what I am using locally... still worth it rebase though. @kinow Any ideas? |
OK, now you can rebase. |
Still seeing the same failure I saw yesterday on |
This PR looks odd, you need to only have your commits in here. Git can be tricky sometimes... |
ahh, I did it through Intellij, should have done it manually... |
😜 time to redo it then |
ok 😁 will close that PR and open a new one, maybe I enhance this change and add parameterized tests 😀 |
You don't need to close this PR and open a new one, this creates noise on the mailing list and disconnects the history of the issue. It's better to keep it here IMO. |
9485476
to
68a1404
Compare
I think it's good RN. Would you mind looking at this @garydgregory, please? |
I kicked off the builds... |
Thank you @garydgregory and @kinow for review 😊 |
I also wonder, can we maybe add junit-jupiter-params artifact to the project, so we can write parameterized tests?
FYI: @kinow @garydgregory