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.

Sign up for GitHub

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

Jump to bottom

Introduce real companion object for Schedule - Fix #1111 #1140

Merged
merged 1 commit into from
Jul 10, 2019
Merged

Introduce real companion object for Schedule - Fix #1111 #1140

merged 1 commit into from
Jul 10, 2019

Conversation

dorsev
Copy link
Contributor

@dorsev dorsev commented Jul 6, 2019

No description provided.

@dorsev dorsev force-pushed the add-companion-object-to-schedule branch from d291107 to 98a1988 Compare July 6, 2019 13:09
ghostdogpr
ghostdogpr reviewed Jul 7, 2019
View reviewed changes
Copy link
Member

@ghostdogpr ghostdogpr left a comment •
edited

Choose a reason for hiding this comment

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

Looks good, just one remark on the scaladoc.
Also for some reason, there are weird errors in the CI for Scala 2.11:
https://circleci.com/gh/zio/zio/4836
https://circleci.com/gh/zio/zio/4841
https://circleci.com/gh/zio/zio/4876

The errors are about RetrySpec and RepeatSpec and I haven't seen them before, so they seem related to this change 🤔

core/shared/src/main/scala/zio/Schedule.scala Outdated
final def recurs(n: Int): Schedule[Any, Int] = ZSchedule.recurs(n)

/**
* See [[ZSchedule.delayed]]
Copy link
Member

Choose a reason for hiding this comment

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

Should be spaced here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch! fixed it

@dorsev dorsev force-pushed the add-companion-object-to-schedule branch from 98a1988 to 64e5660 Compare July 7, 2019 10:05
@dorsev
Copy link
Contributor Author

dorsev commented Jul 7, 2019 •
edited

@ghostdogpr, regarding the failing tests I agree with you.
Would love any help debugging this since the tests are passing on my computer(and are passing CI occasionally).
I Think the timeout originates from line 25 in BaseCrossPlatformSpec.scala. Any Idea what does this timeout mean?

@dorsev dorsev force-pushed the add-companion-object-to-schedule branch 3 times, most recently from 1f9c33e to fd08c6b Compare July 7, 2019 20:00
@dorsev dorsev closed this Jul 7, 2019
@dorsev dorsev reopened this Jul 7, 2019
@dorsev dorsev force-pushed the add-companion-object-to-schedule branch from fd08c6b to 25f868e Compare July 7, 2019 20:24
@dorsev
Copy link
Contributor Author

dorsev commented Jul 7, 2019 •
edited

EDIT:

  1. @ghostdogpr I Fixed the failing tests! it was a cyclic reference from 'ZSchedule' to 'Schedule' which pointed to 'ZSchedule'.
    the tests now pass consistently on my computer and on this build server 🙏

  2. when I was re-running the tests locally 10-20 times I found another inconsistent test that keeps on popping(at least on my local machine)
    get must fail if no value is set in OneShotSpec.scala
    for example
    [error] 14391574803 is not less than 10200000000 (OneShotSpec.scala:39)
    Should I open a bug on this? or is it just me?

@dorsev dorsev force-pushed the add-companion-object-to-schedule branch 3 times, most recently from ffc819e to 120d7e8 Compare July 8, 2019 16:18
@dorsev dorsev force-pushed the add-companion-object-to-schedule branch from 120d7e8 to 91f0671 Compare July 8, 2019 19:46
@ghostdogpr ghostdogpr merged commit 2da29aa into zio:master Jul 10, 2019
@ghostdogpr
Copy link
Member

@dorsev thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ghostdogpr ghostdogpr ghostdogpr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@dorsev @ghostdogpr