eclipsesource / eclipsesource/cdtcloud-deploymentserver
Review Feedback
- Dominant language
- TypeScript
- Stars
- 4
- Forks
- 4
- PR merge metrics
- No merged PRs in 30d
Description
Hi guys,
I took a look at your code, here are my remarks.
- add readme files to each sub package
- add useful extensions to the list of recommended extensions
### deployment-server-ui
#### redux
- co-locate all redux stuff in one folder (reducers are next to the app folder which only contains the store)
- check whether your redux state is enough, especially the boolean flags like loading, maybe rather use a status with 3 states: 'loading', 'failed', 'successful'
- maybe use more of the redux/toolkit classes eg entities and thunks for retrieving the dashboard data: see https://redux-toolkit.js.org/api/createAsyncThunk, you could get rid of the `setError` and `setData` reducers that feel weird
- use optional instead of null
- maybe move data loaded in components into redux? thus also handle states etc there
#### generic
- remove commented out code
- double check class and interface naming, there is no need to prefix interfaces with `I`, some types are lowercase
- use push api on backend instead of pulling every 1-3 seconds
- move the code to App.tsx and directly use the Dashboard for the `/` path ?
- why use an identity function (defineFunctionalComponent) to wrap the functional component?
- split big components in smaller one eg the dashboard
- use optional chaining (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining)
- use arrow functions (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions)
- use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment to directly access the properties you later need of an object
- maybe pull out common formatter functions to a common util
- rename 'services' to 'hooks' as those are react hooks
### device-connector
- why the `#` prefix in private members
### grpc
- why the partial copy?
### theia-extension
- why the empty registerMenu registration?
- why the mix of got and axios in backend?
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.