Skip to content
This repository has been archived by the owner on Feb 1, 2023. It is now read-only.

Add missing godoc comments, refactor to avoid confusion #117

Merged
merged 2 commits into from
May 10, 2019

Conversation

hannahhoward
Copy link
Contributor

Goals

This is the first of probably a few PRs to continue bitswap cleanup -- just clearing up ongoing areas of confusion and reducing accumulated tech debt. This immediate PR is simply to solve #101 -- improve the godocs and clear up confusion due to naming conflicts

Implementation

  • Add missing comments to many exported functions -- this is the source of most of the file changes in this PR
  • Extract code in testutils.go to its own module -- this was simply the code for building complete test instances of bitswap for the tests in the root directory (which are effectively integration tests)
  • Make integration tests have package name bitswap_test, to avoid import cycle and always because they are bitswap integration tests (of all the packages in the bitswap module), which means they ought to only access public bitswap interfaces anyway
  • there was one unused constant that was removed (rebroadcastDelay -- was used by old pre-session bitswap code)
  • Rename the SessionGenerator class -- this class had nothing to do with Bitswap sessions, but rather creating test instances of Bitswap for testing purposes. This caused people to look to generate bitswap sessions with it, in part cause bitswap.NewSession was not documented. (see Godocs, particularly about NewSession() #101)

@ghost ghost assigned hannahhoward May 10, 2019
@ghost ghost added the status/in-progress In progress label May 10, 2019
@hannahhoward hannahhoward requested review from michaelavila, hsanjuan, Stebalien and eingenito and removed request for michaelavila May 10, 2019 00:14
@@ -28,7 +36,8 @@ type peerQueueInstance struct {
// PeerManager manages a pool of peers and sends messages to peers in the pool.
type PeerManager struct {
// peerQueues -- interact through internal utility functions get/set/remove/iterate
peerQueues map[peer.ID]*peerQueueInstance
peerQueues map[peer.ID]*peerQueueInstance
peerQueuesLk sync.RWMutex
Copy link
Member

Choose a reason for hiding this comment

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

Did we need this for a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, no that's a mistake. will remove.

@hsanjuan
Copy link
Contributor

Thanks, I'll have a closer look in the following days and give some feedback if needed.

Add comments to all exported functions, extract the utils for creating instances in testnet.go,
moves integration tests to bitswap_test

BREAKING CHANGE: removed one constant -- rebroadcastDelay -- which I believe was unused
Instance generator was previously named session generator, which created confusion with bitswap
sessions

fix #101
@Stebalien Stebalien merged commit 5ec87a8 into master May 10, 2019
@Stebalien Stebalien removed the status/in-progress In progress label May 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants