Skip to content

Conversation

mbonneau
Copy link
Member

No description provided.

Copy link
Member

@asm89 asm89 left a comment

Choose a reason for hiding this comment

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

LGTM!

.travis.yml Outdated

after_success:
- travis_retry php vendor/bin/coveralls -v
- travis_retry php vendor/bin/coveralls -v
Copy link
Member

Choose a reason for hiding this comment

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

nit: revert?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't remember removing that. I will take a look.

Comment on lines +80 to +81
// allow demos without expect files - we will just get a warning later
// assert(file_exists($outputPath), "output does not exist $outputPath");
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to be removed? Perhaps we can skip and print a warning for now if the output path does not exist?

Copy link
Member Author

Choose a reason for hiding this comment

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

In PHP 7 this generated a warning and the tests still passed. With PHP 8 this generates a fatal error and stops.

A warning is still generated later when file_get_contents is called, but the tests still work.

* @param callable $comparer Comparer to determine causality of events based on absolute time.
*/
public function __construct(int $initialClock = 0, callable $comparer)
public function __construct(int $initialClock, callable $comparer)
Copy link
Member

Choose a reason for hiding this comment

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

Interesting. I guess this went unnoticed before.

@mbonneau mbonneau merged commit f196569 into ReactiveX:master Dec 13, 2020
@mbonneau mbonneau deleted the testing_updates branch December 13, 2020 02:17
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.

3 participants