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

who monitors the monitor #1488

Merged
merged 3 commits into from
Apr 24, 2020
Merged

who monitors the monitor #1488

merged 3 commits into from
Apr 24, 2020

Conversation

InoMurko
Copy link
Contributor

@InoMurko InoMurko commented Apr 24, 2020

If you want to receive exit signals from processes you need to monitor them. I don't see any code implying that RootChainCoordinator monitors Ethereum Event Listeners and these two chunks of code would ever get called. So I'm removing them.

@coveralls
Copy link

coveralls commented Apr 24, 2020

Coverage Status

Coverage decreased (-0.002%) to 77.905% when pulling f0d1547 on inomurko/emmm_wut into 4c5dfd8 on master.

@InoMurko InoMurko marked this pull request as ready for review April 24, 2020 10:41
@@ -186,7 +186,7 @@ jobs:
_counter=$(mix credo --only Credo.Check.Readability.SinglePipe | grep -c "Use a function call when a pipeline is only one function long")
echo "Current Credo.Check.Readability.SinglePipe occurrences:"
echo $_counter
if [ $_counter -gt 367 ]; then
if [ $_counter -gt 359 ]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

bravo team!!!

@unnawut
Copy link
Contributor

unnawut commented Apr 24, 2020

This means that after this PR we would see the listener crashes in the logs?

@InoMurko
Copy link
Contributor Author

No. This changes nothing. Coordinator does not monitor listeners at all. I can’t find any code that would invoke monitoring.

@unnawut
Copy link
Contributor

unnawut commented Apr 24, 2020

No. This changes nothing. Coordinator does not monitor listeners at all. I can’t find any code that would invoke monitoring.

Aha ok got it!

@InoMurko InoMurko merged commit 8189b81 into master Apr 24, 2020
@InoMurko InoMurko deleted the inomurko/emmm_wut branch April 24, 2020 13:41
@unnawut unnawut added the enhancement New feature or request label Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants