Details
- Reviewers
ashoat atul ginsu - Commits
- rCOMMbccb9fff1e58: Refactor lib/facts files from JSON to JS
Both Web and Native seem functional to me. Messages still load as expected, and created several new accounts to test the Genesis bot (results are identical with these changes and on the current master branch).
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Initial Commit that deals with Eng 1181, upgrading Node version and removing json imports
Changed import json to use fs instead of import (ENG-929)
In order to upgrade node version, we must stray away from experimental features like import for json.
looks like I was added because the initial diff made some changes to native/cpp, going to resign, as I'm not qualified to review this
| keyserver/src/scripts/add-edit-thread-detailed-permissions.js | ||
|---|---|---|
| 1 ↗ | (On Diff #17015) | Add newline after // @flow |
| lib/facts/ashoat.js | ||
| 1 ↗ | (On Diff #17015) | Add newline after // @flow |
| 3–5 ↗ | (On Diff #17015) | $ReadOnly |
| lib/facts/bots.js | ||
| 1 ↗ | (On Diff #17015) | Add newline after // @flow |
| 2–4 ↗ | (On Diff #17015) | $ReadOnly |
| 3 ↗ | (On Diff #17015) | Fix any type |
| lib/facts/genesis.js | ||
| 1 ↗ | (On Diff #17015) | Add newline after // @flow |
| 2–7 ↗ | (On Diff #17015) | $ReadOnly |
| 6 ↗ | (On Diff #17015) | $ReadOnlyArray |
| lib/facts/staff.js | ||
| 1 ↗ | (On Diff #17015) | Add newline after // @flow |
| lib/facts/testers.js | ||
| 1 ↗ | (On Diff #17015) | Add newline after // @flow |
| 2–6 ↗ | (On Diff #17015) | $ReadOnly |
| 5 ↗ | (On Diff #17015) | $ReadOnlyArray |
Refactor $ReadOnly< ... > syntax to be more consistent with the codebase and separate ENG-1182 changes out of this diff (following sync with Atul)
Great work!! This diff looks clean, and I'm glad you separate out the changes to import-json.js to a different diff.
One thing that is new from when we created the Linear issue: we now have lib/shared/comm-icon-config.json and lib/shared/swmansion-icon-config.json, which are both being accessed from both web and native. I think things should be fine as long as keyserver doesn't import the web file directly... and those will be annoying to migrate since they're auto-generated. But figured I'd note it here. cc @atul who I think initially introduced both
Thanks for the heads up! I'll land this to not block it, but if those two JSON files do become an issue, I'm happy to try to deal with it in a separate diff