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

We need a break! #83

Merged
merged 3 commits into from
Aug 1, 2019
Merged

We need a break! #83

merged 3 commits into from
Aug 1, 2019

Conversation

chmanie
Copy link
Member

@chmanie chmanie commented Aug 1, 2019

This PR adds a break statement for the ANNOUNCE_CLIENT case to not fall through to the PIN_HASH case.

Also we're sneaking in a commit to improve the buffer logging in debug mode

@chmanie chmanie added bug Something isn't working needs-code-review labels Aug 1, 2019
@chmanie chmanie self-assigned this Aug 1, 2019
}
case PIN_HASH: {
if (!ipfsHash) {
log('PIN_HASH: no ipfsHash given: %O', message.data);
if (log.enabled) {
// Only stringify when absolutely necessary
Copy link
Member

Choose a reason for hiding this comment

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

Can we add this to the REPLICATE case below and the 'could not parse pinner message' error above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, yeah sure, I missed those!

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

src/Pinion.ts Outdated
} catch (caughtError) {
if (log.enabled) {
// Only stringify when absolutely necessary
log('Could not parse pinner message: %O', message.data);
Copy link
Member

@area area Aug 1, 2019

Choose a reason for hiding this comment

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

This doesn't even stringify when absolutely necessary 😛

EDIT: Nor does the REPLICATE one

Copy link
Contributor

@JamesLefrere JamesLefrere left a comment

Choose a reason for hiding this comment

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

Looks sorted to me 👍

@chmanie chmanie merged commit 9d2c9f6 into master Aug 1, 2019
@chmanie chmanie deleted the fix/catch-a-break branch August 1, 2019 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working under-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants