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

Guava TableAssert#isNotEmpty() added #3559

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

mk868
Copy link

@mk868 mk868 commented Aug 6, 2024

The TableAssert currently contains isEmpty() assertion, but no isNotEmpty(). I want to extend this to be like other collection assertions.

Check List:

Changes in this PR:

  • A new assertion isNotEmpty() added to the TableAssert class
  • Added unit tests for TableAssert#isNotEmpty()
  • Fixed typo in the TableAssert_isEmpty_Test#should_fail_if_actual_is_null() test method

@mk868 mk868 force-pushed the guava-assertions branch 2 times, most recently from 8c0700b to 71a8909 Compare August 9, 2024 19:25
@mk868
Copy link
Author

mk868 commented Aug 9, 2024

Branch rebased

@mk868 mk868 changed the title Guava TableAssert#isNotEmpty() added Guava TableAssert#isNotEmpty() added Aug 19, 2024
@mk868 mk868 force-pushed the guava-assertions branch 2 times, most recently from e37628a to eaf90c3 Compare August 19, 2024 15:32
@mk868
Copy link
Author

mk868 commented Aug 19, 2024

Branch rebased

@mk868
Copy link
Author

mk868 commented Sep 5, 2024

Is there any chance to merge this?
Or should I create an issue with a better described motivation of this change?

@scordio
Copy link
Member

scordio commented Sep 5, 2024

Hi @mk868, sorry for the slow feedback! I just let the pipeline run, you might face some formatting alerts that can be fixed with spotless.

Anyway, I'll jump on the PR over the weekend and get back to you.

@scordio scordio self-assigned this Sep 5, 2024
@scordio scordio added this to the 3.27.0 milestone Sep 5, 2024
Copy link
Member

@scordio scordio left a comment

Choose a reason for hiding this comment

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

Just a few cosmetic comments. No worries if you don't have time, I can take care of them during the merge.

@mk868
Copy link
Author

mk868 commented Sep 5, 2024

Thank you for your review!
I applied your suggestions, and also run spotless reformatting

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