Replacement for bad pull request on branch separate-NETWORKS #143
Conversation
Bill-Kunj
changed the title
There was a problem hiding this comment.
"Replacement for branch separate-NETWORKS" is phrased in terms of ephemera (a branch name). Commit messages are part of the permanent history of the project.
Please use a commit message that explains the purpose of the commit. I don't understand the purpose of this commit / PR.
But I found at least one or two things that should change.
| .catch((err) => { | ||
| console.log(proposeCount, err); | ||
| if (proposeCount < 7) { | ||
| setTimeout(propose, 10000); |
There was a problem hiding this comment.
setTimeout is ambient authority.
| * @param { string } adm | ||
| * @returns {net} | ||
| */ | ||
| function makeNetwork(pattern, obs, val, adm) { |
There was a problem hiding this comment.
what's the purpose of the makeNetwork function? It doesn't seem to have any benefit over writing out the object literal.
| listenAtDeployId, | ||
| } from 'rchain-api'; | ||
|
|
||
| import m from 'mithril'; |
There was a problem hiding this comment.
m includes ambient authority, IIRC.
| /** | ||
| * @typedef {string} hostname | ||
| * | ||
| * @type {{ localhost: net, testnet: net, demo: net, rhobot: net, mainnet: net}} NETWORKS |
There was a problem hiding this comment.
the syntax is:
/** @type {...} */
const NETWORKS = ...| Object.entries(NETWORKS).find( | ||
| ([_name, net]) => { | ||
| if (hostname.indexOf(net.hostPattern) >= 0) return net; | ||
| }) |
There was a problem hiding this comment.
What is this call to Object.entries supposed to do? It looks like a noop.
There was a problem hiding this comment.
Completely agree, I don't actually know what that code was written for
| /** | ||
| * @returns {string[]} | ||
| */ | ||
| export function networkNames() { |
There was a problem hiding this comment.
networkNames might make sense if the purpose were to abstract the representation of NETWORKS, but NETWORKS is exported, so I don't understand what this function is for.
|
Hi @dckc, thanks for this review. Indeed I think there has been a misunderstanding on the purpose of this PR. First, I'd like to say that none of the code blocks that you commented was written by me. All I did was fixing a bunch of bugs on this branch, after @Bill-Keuntje let us know it would be interesting to include the refactoring work of this branch into our branch ballot-ui. And honestly I'm not sure I am the best person to do this refactoring work as I just discovered the code and that there are still a lot of things I don't quite understand. Maybe the person who started this refactoring ? Do you think there is still a lot to do ? |
|
This PR is a first step to allow multiple UIs to be developed for various rgov projects/features using the same code and at the same time avoid the inevitable conflicts/dependencies in participate.js (which is the development interface). |
|
@dckc @jimscarver @w2vy @David405 @pfabre31 |
|
Given no active response, this is abandoned. Opening new issue to refactor participate.js #217 |
Move deploy, propose, and NETWORKS into individual files.