Open tcodes0 opened 4 years ago
~probably merge root tsconfig with tsconfig.base, they are doing the same thing really~ nevermind, you seem to know what you're doing with TS
https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/app.ts#L1-L9 I really like the use of absolute imports š
really like comments like this too https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/config.ts#L66-L68
avoid sync fns in node fs, try using the async one with fs.promises or util.promisify https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/config.ts#L34
sync
fs
Try refactoring this class to functions. https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/app.ts#L11 you can use scoping to create isolation like class member accessibility does in a class
nice use of decorators for graphql, do angular background? https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/resolvers/approval.ts#L43-L59
pretty solid typescript, congrats
nice https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/front/data/schema.graphql#L85-L89 do you prefer schema-first graphql approaches? we use code first lol
dumb rule, I dont use it https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/front/src/relay.env.ts#L27 but I do use a rule that says you need to document ts-ignore to explain why you're ignoring that thing
see #7 #5 #4 #3 š
for tests try to unit test the backend and integration test the front for CI run your typechecker, build and tests also add precommit hooks https://github.com/FotonTech/golden-stack/blob/master/husky.config.js and prettier for formatting https://github.com/FotonTech/golden-stack/blob/master/.prettierrc.js
node interface is neat too https://github.com/FotonTech/golden-stack/tree/master/packages/server/src/interface
overall we have some cool relay stuffs here https://github.com/FotonTech/golden-stack/blob/master/packages/relay-web/src/mutationUtils.tsx try to incorporate in some way
:)
Thank you soo much for the detailed feedback. I will do the changes the things you pointed out. It's my first app with TS and Graphql. I liked so much this stack and i will study deeply.
~probably merge root tsconfig with tsconfig.base, they are doing the same thing really~ nevermind, you seem to know what you're doing with TS
https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/app.ts#L1-L9 I really like the use of absolute imports š
really like comments like this too https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/config.ts#L66-L68
avoid
sync
fns in nodefs
, try using the async one with fs.promises or util.promisify https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/config.ts#L34Try refactoring this class to functions. https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/app.ts#L11 you can use scoping to create isolation like class member accessibility does in a class
nice use of decorators for graphql, do angular background? https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/server/src/resolvers/approval.ts#L43-L59
pretty solid typescript, congrats
nice https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/front/data/schema.graphql#L85-L89 do you prefer schema-first graphql approaches? we use code first lol
dumb rule, I dont use it https://github.com/PhilipeGatis/approval/blob/51fe94c88d79bf0664e71b3397d4e0808e1da1ba/packages/front/src/relay.env.ts#L27 but I do use a rule that says you need to document ts-ignore to explain why you're ignoring that thing
see #7 #5 #4 #3 š
for tests try to unit test the backend and integration test the front for CI run your typechecker, build and tests also add precommit hooks https://github.com/FotonTech/golden-stack/blob/master/husky.config.js and prettier for formatting https://github.com/FotonTech/golden-stack/blob/master/.prettierrc.js
node interface is neat too https://github.com/FotonTech/golden-stack/tree/master/packages/server/src/interface
overall we have some cool relay stuffs here https://github.com/FotonTech/golden-stack/blob/master/packages/relay-web/src/mutationUtils.tsx try to incorporate in some way
:)