Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: codebase cleanup #509

Merged
merged 39 commits into from
Jul 1, 2022
Merged

Refactor: codebase cleanup #509

merged 39 commits into from
Jul 1, 2022

Conversation

mattyg
Copy link
Collaborator

@mattyg mattyg commented Jun 30, 2022

Overview

Docs

  • docstrings for all api functions
  • docstrings for all discord-bot functions

Refactor

  • move period/controllers/assignments non-controller functions into period/utils/assignments
  • remove redundant functions: praiseWithScore
  • modify InlineLabelClosable to use InlineLabel and use in Praise component, remove no longer needed ResetQuantificationButton
  • remove unncessary transformer wrapper functions, standardize function naming
  • check if umzug migrations are complete before running migrations
  • generate praise _id labels (i.e. #abcd) in backend, pass to frontend via PraiseDto
  • add types to express Request when expecting parameters in Request.body
  • modify getRandomString to use more secure crypto.randomBytes

Build

  • delete and regenerate yarn.lock file to upgrade packages and resolve conflicts
  • confirm discord.js function Util.cleanContent works with updated packages
  • add placeholder .eslintrc to discord-bot

Cleanup

  • search for and remove all "TODO" comments -- move to github tickets
  • delete unused functions
  • remove unused exports
  • add eslint rule for import ordering, and standardize import orders
  • run eslint on frontend build
  • re-add eslint rule import/no-default-exports and update all exports / imports to reflect. This ensures that all imported functions are named identically as when they are exported, which I think gives more clarity to devs.
  • standardize typescript path aliases formatted as @/blah and @blah between api and frontend -- now both are @/blah

mattyg added 30 commits June 28, 2022 18:49
…e InlineLabelClosable in Praise component, remove unneeded ResetQuantificationButton
… force all imports to have identical names to their original function name. Update all function exports and imports to reflect this rule (except for those loaded with React.lazy which only supports default exports)
…' for consistency with frontend path aliases
…flict.

Fix new build errors arising from dependency minor updates.
Ignore one build error I was unable to diagnose.
…is missing from /praise or /forward commands
…ormat()' and change formatting to iso standard yyyy-mm-dd
@mattyg mattyg changed the title Refactor/codebase cleanup Refactor: codebase cleanup Jun 30, 2022
@mattyg
Copy link
Collaborator Author

mattyg commented Jun 30, 2022

No idea why the github actions aren't running -- maybe because there are so many file changes the trigger checks are timing out?

Anyway you might need to run a lint, build and test locally to confirm.

@mattyg mattyg requested a review from kristoferlund June 30, 2022 21:19
Copy link
Member

@kristoferlund kristoferlund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a needed cleanup, great, thanks! ✨

@@ -10,7 +10,7 @@ export enum EventLogTypeKey {
SETTING = 'SETTING',
}

export interface EventLog {
interface EventLog {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great not to export these types! Avoids accidental use.

@kristoferlund kristoferlund merged commit 2860161 into main Jul 1, 2022
@kristoferlund kristoferlund deleted the refactor/codebase_cleanup branch July 1, 2022 12:07
@kristoferlund kristoferlund mentioned this pull request Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants