Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

feat: add a transports section for enabling/disabling transports #102

Merged
merged 11 commits into from
Jun 16, 2020

Conversation

Stebalien
Copy link
Member

No description provided.

@Stebalien Stebalien requested a review from aschmahmann June 15, 2020 20:42
These let us zero-encode "default" to "null" (and omit it with "omitempty") so
we don't have to hard code the default in the config.
This was never included in a release, and has been replaced by a flexible
transport prioritization system.
Copy link
Contributor

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

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

Left a bunch of questions and some doc corrections, but overall LGTM. Thanks for all the doc strings and cleaning this up 🙏

swarm.go Show resolved Hide resolved
swarm.go Outdated Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
swarm.go Outdated Show resolved Hide resolved
experiments.go Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
Copy link
Member Author

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

General theme: I don't want to deal with a bunch of error cases. What do you think about panicing in all of these cases? They're all programmer error.

types.go Outdated Show resolved Hide resolved
types.go Outdated Show resolved Hide resolved
It's not a part of the JSON spec.
@aschmahmann
Copy link
Contributor

I think panicing seems like a reasonable approach as long as the valid/invalid cases are documented. It does mean putting in the error detection code for when to panic, but that's probably worthwhile.

@Stebalien Stebalien merged commit 9a18412 into master Jun 16, 2020
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.

2 participants