Skip to content

bitswap/README.md: Cleaning up the format #168

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

Merged
merged 1 commit into from
Dec 13, 2017

Conversation

dgrisham
Copy link
Member

Going to start updating this document soon, so I figured I'd handle the reformatting before I made any content changes to keep commits/updates clean.

@dgrisham
Copy link
Member Author

@diasdavid

Bitswap is the data trading module for ipfs, it manages requesting and sending blocks to and from other peers in the network. Bitswap has two main jobs, the first is to acquire blocks requested by the client from the network. The second is to judiciously send blocks in its posession to other peers who want them.

Bitswap is the data trading module for ipfs, it manages requesting and sending
blocks to and from other peers in the network. Bitswap has two main jobs, the
Copy link
Member

Choose a reason for hiding this comment

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

Keeping it one line is fine :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Gotcha, I'll change that back. Kind of unfortunately painful to deal with for my text editor, but I'll get by :)

Copy link
Member

Choose a reason for hiding this comment

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

@dgrisham which editor are you using?

Copy link
Member Author

Choose a reason for hiding this comment

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

Kakoune -- it has visual soft-wrapping, but unfortunately it still behaves like a single line when you use that feature. It's a young text editor, so these things come with the territory. I just made a Bash function that uses pandoc to convert the input file to one with 80-character lines, then once I exit the text editor it converts it back so that the paragraphs are all on one line again (as it is here).

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

The rest LGTM

}

optional Wantlist wantlist = 1;
repeated bytes blocks = 2;
}
Copy link
Member

Choose a reason for hiding this comment

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

Why remove the ```?

# Implementation details

Also, make sure to read - https://github.com/ipfs/go-ipfs/tree/master/exchange/bitswap#go-ipfs-implementation
Also, make sure to read - <https://github.com/ipfs/go-ipfs/tree/master/exchange/bitswap#go-ipfs-implementation>
Copy link
Member

Choose a reason for hiding this comment

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

No need for angle brackets

// 2) if not -> ask bitswap
// 2.1) bitswap will cb() once the block is back, once. bitswap will write to the repo as well.
})
```
Copy link
Member

Choose a reason for hiding this comment

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

You should not remove the ```, they make the code rendered with sintax highlighting

@dgrisham
Copy link
Member Author

@diasdavid ACK on those comments -- when I used pandoc to handle the line length thing, it also reformatted other parts of the document. I should've caught that, I'll rework my solution and push once it's not doing those unnecessary rewrites.

@dgrisham dgrisham force-pushed the fmt/reformatting branch 4 times, most recently from 0f19c3e to 6a952da Compare December 12, 2017 21:47
@dgrisham
Copy link
Member Author

@diasdavid Hey, finally got back to this, let me know what you think.


# Status of this spec
Status of this spec
===================
Copy link
Member

Choose a reason for hiding this comment

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

Why switch from # to ====?

- c) Send it.
1. Ensure it has a connection to its peer
2. Grab the message to be sent
3. Send it
Copy link
Member

Choose a reason for hiding this comment

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

This will not auto enum in Github flavoured markdown

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you sure? Seems to indicate that it will on the spec here, at least if I'm understanding it correctly.

Copy link
Member

Choose a reason for hiding this comment

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

You are right! :)

- bitswap-decision-engine
- bitswap-message
- bitswap-net
- bitswap-wantlist
Copy link
Member

Choose a reason for hiding this comment

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

Too many spaces

- bitswap.peerConnected(peer)
- add peer to peer set + send them wantlist (maybe)
- bitswap.peerDisconnected(peer)
- remove peer from peer set
Copy link
Member

Choose a reason for hiding this comment

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

Too many spaces

@dgrisham
Copy link
Member Author

dgrisham commented Dec 13, 2017

@diasdavid Updated again -- the brackets around links are there again, because pandoc uses Github's CommonMark writer and they have that as part of their specification.

Edit: To be more clear about that: everything in this version of the document is exactly what Github's CommonMark writer outputs, except for the codeblocks that you pointed out before -- I edited pandoc a bit to put the ``` around codeblocks rather than indenting, I agree that that's better here.

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Thanks ! :)

@dgrisham
Copy link
Member Author

@diasdavid Thanks David! Would you mind merging? I don't have permissions to do so.

@daviddias daviddias merged commit 342b373 into ipfs:master Dec 13, 2017
@dgrisham dgrisham deleted the fmt/reformatting branch December 13, 2017 18:06
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.

2 participants