Closed dominictarr closed 4 years ago
Two thoughts:
On Mon, 21 Jan 2019, 00:04 Dominic Tarr, notifications@github.com wrote:
@mixmix https://github.com/mixmix I want to suggest this change. when I merged your earlier PR, I changed something: #31 https://github.com/ssbc/ssb-config/pull/31
I noticed that utils/get-net and utils/get-ws where nearly the same except that get-net iterated over all net protocols, but get-ws only took the first one. I figured surely this was an oversight - one of the problems we have with configuration is making sure things are handled in a consistent logical way, so the port that is being used for ws and net ought to be detected in the same way.
But, on the other hand, less abstract code is easier to read, because you just read it top to bottom...
But on the other hand, when things are the same, if they use the same code, then that strongly emphasizes that those things are the same. I think that makes it easier to maintain, because you can't change one thing without changing the other too. That way things intended to be the same change together.
What do you think?
You can view, comment on, or merge this pull request online at:
https://github.com/ssbc/ssb-config/pull/34 Commit Summary
- remove duplication
File Changes
- M util/fix-connections.js https://github.com/ssbc/ssb-config/pull/34/files#diff-0 (31)
- D util/get-net.js https://github.com/ssbc/ssb-config/pull/34/files#diff-1 (20)
- D util/get-ws.js https://github.com/ssbc/ssb-config/pull/34/files#diff-2 (20)
Patch Links:
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ssbc/ssb-config/pull/34, or mute the thread https://github.com/notifications/unsubscribe-auth/ACitnjKCLPNbsZyR0Fhpoto0mghJ8gVdks5vFE0hgaJpZM4aJlap .
@mixmix prior to this ssb-server worked without a default port set. because the port is defaulted to somewhere else.
I think the removing duplication aspect is more important: getting ws port should be done the same way as getting the net port, deduplicating code gaurantees that. duplicating code makes it easy to do it differently by accident
@mixmix local should be synonym for private. actually "local" is preferred by secret-stack https://github.com/ssbc/secret-stack/blob/master/core.js#L87
"private" is weird in that it means a local network... it might not be very private if it's it's say a cafe. 'public' is also unfortunate. but other code already expects "local"
Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?
@dominictarr is this still relevant? Also, seems like the definition of local / private have evolved? Or is there a mixup here?
@mixmix I want to suggest this change. when I merged your earlier PR, I changed something: https://github.com/ssbc/ssb-config/pull/31
I noticed that
utils/get-net
andutils/get-ws
where nearly the same except thatget-net
iterated over all net protocols, butget-ws
only took the first one. I figured surely this was an oversight - one of the problems we have with configuration is making sure things are handled in a consistent logical way, so the port that is being used for ws and net ought to be detected in the same way.But, on the other hand, less abstract code is easier to read, because you just read it top to bottom...
But on the other hand, when things are the same, if they use the same code, then that strongly emphasizes that those things are the same. I think that makes it easier to maintain, because you can't change one thing without changing the other too. That way things intended to be the same change together.
What do you think?