issue: ENG-6615
Details
I added node $PRJ_ROOT/scripts/set-admin-data.js to dev-shell.nix:
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
scripts/set-admin-data.js | ||
---|---|---|
1 ↗ | (On Diff #37075) | I don't think Flow is actually checking this. We don't have a Flow workspace configured at the repo root. We do already have four other JS scripts in this folder, all of which use the Flow pragma, and none of which are actually using Flow. In fact I think if you tried to add Flow typehints, it would prevent this script from running directly with Node. Two requests:
|
33–57 ↗ | (On Diff #37075) | Should we do any error-checking here? |
scripts/set-admin-data.js | ||
---|---|---|
17–18 ↗ | (On Diff #37075) | Should we hide the password in the console? Shouldn't be too hard https://stackoverflow.com/questions/24037545/how-to-hide-password-in-the-nodejs-console, but it's just for the local dev environment, so not necessarily worth it. |
scripts/set-admin-data.js | ||
---|---|---|
1 ↗ | (On Diff #37075) |
We took this approach because the script will be taking the user input. And if I'm not mistaken, when the keyserver is run, a couple of threads run at one printing to the output, so that could cause problems.
I would rather do this as a followup, because a couple of devs need to be able to use multiple keyservers, and I will be gone next week. I created a task in my March goal that is the second part of this testing goal, but if you think this is a priority, please let me know |
17–18 ↗ | (On Diff #37075) | I don't think it's worth doing now, but if you feel strongly about this, let me know |
33–57 ↗ | (On Diff #37075) | I think if creation of these files fail, than nix develop should just fail, to force a re-run. |
I would rather do this as a followup, because a couple of devs need to be able to use multiple keyservers, and I will be gone next week. I created a task in my March goal that is the second part of this testing goal, but if you think this is a priority, please let me know
This would take ~3min... I wonder if the amount of time you spent typing this message and creating the task, along with the time I am now spending typing this, is higher than the amount of time it takes you to simply put a diff up that strips the annotations. In this case I don't think the work should have been deferred. It's less a question of prioritization, and more a question of using your time and my time wisely.
scripts/set-admin-data.js | ||
---|---|---|
1 | Can you please strip this before landing? | |
17–18 ↗ | (On Diff #37075) | It seems fairly trivial to do this now... but if there's a place that a follow-up task should have been created, it's definitely here. This is directly related to the project and would probably take you something more like 15-30min |
This would take ~3min... I wonder if the amount of time you spent typing this message and creating the task, along with the time I am now spending typing this, is higher than the amount of time it takes you to simply put a diff up that strips the annotations. In this case I don't think the work should have been deferred. It's less a question of prioritization, and more a question of using your time and my time wisely.
Simply stripping the notations causes eslint errors, so I would have to figure out how to deal with that
scripts/set-admin-data.js | ||
---|---|---|
1 | This causes eslint errors | |
17–18 ↗ | (On Diff #37075) | Here is the task: ENG-6874 |
Remove flow annotation. I was informed this is how this should be done, hope this is fine