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

Add matchers to Klock. #1214

Merged
merged 12 commits into from
Feb 17, 2020
Merged

Add matchers to Klock. #1214

merged 12 commits into from
Feb 17, 2020

Conversation

dragneelfps
Copy link
Contributor

Related to #1156.

@dragneelfps
Copy link
Contributor Author

@sksamuel I have created this draft. I wanted to get an opinion on the approach I have used. Also, what are the matchers that should be implemented.

@LeoColman LeoColman self-requested a review February 11, 2020 19:16
@LeoColman
Copy link
Member

LeoColman commented Feb 11, 2020

I believe you're on the right track.
I recommend adding some matchers for numbers, like Time.shouldHaveMinutes(5) or something similar.

I'm not sure if you're using our current datetime matchers as inspiration, but that can be a good starting point if you feel lost! I believe you are doing things that way already tho

Take a look

@dragneelfps dragneelfps marked this pull request as ready for review February 11, 2020 21:05
@dragneelfps
Copy link
Contributor Author

@Kerooker I think I have added all the required matchers. I haven't documented the matchers like done in existing code since, I didnt find any need for doing it, as its self documenting. But if you want me to add docs, then I can.
I have added some tests as well.
I dont know if publishing works on that. Need to check on that.
Other than that, I think the PR is ready to be reviewed.

@ashishkujoy ashishkujoy self-requested a review February 14, 2020 04:29
@sksamuel
Copy link
Member

If you can address the feedback @dragneelfps I will merge. This will be an excellent addition.

@dragneelfps
Copy link
Contributor Author

@sksamuel feedback taken and code changes done.

@dragneelfps
Copy link
Contributor Author

Build failing with:

* Where:
Build file '/home/runner/work/kotest/kotest/kotest-assertions/kotest-assertions-klock/build.gradle.kts' line: 70

* What went wrong:
Could not read script '/home/runner/work/kotest/kotest/publish.gradle' as it does not exist.

Any help?

@sksamuel
Copy link
Member

The build was changed over the weekend so publish.gradle doesn't exist. That's ok, we can merge.

@sksamuel sksamuel merged commit dc9ff45 into kotest:master Feb 17, 2020
@LeoColman LeoColman mentioned this pull request Feb 17, 2020
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.

4 participants