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 const, enum, interface & dto definitions to a common component #510

Merged

Conversation

JoeCap08055
Copy link
Contributor

@JoeCap08055 JoeCap08055 commented Sep 11, 2024

Closes #450

  • Eliminate duplicate constants
  • No logic changes; mostly just file move/rename, update imports

Note, this is probably the largest of a number of planned refactoring PRs, due to the large number of files being moved. Future PRs will focus on a single module/service to refactor, and so should have a smaller impact in number of files.

Copy link
Contributor

@aramikm aramikm left a comment

Choose a reason for hiding this comment

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

Great work! I added some questions and nits. a few things

  • I wished we've done this service by service so it would be easier to test and evaluate but since this is already done doesn't make sense to split the PRs. I think going forward it might make sense to do these service by service.
  • I think renaming AnnouncementTypeDto to AnnouncementTypeName while is nice, it made this PR bigger than it already is and increased the risk on breaking functionality.

.prettierignore Outdated Show resolved Hide resolved
Docker/Dockerfile.content-publishing Show resolved Hide resolved
apps/account-api/src/metadata.ts Show resolved Hide resolved
libs/types/src/constants/index.ts Show resolved Hide resolved
libs/types/src/constants/queue.constants.ts Outdated Show resolved Hide resolved
apps/content-publishing-api/src/metadata.ts Show resolved Hide resolved
apps/content-watcher/src/metadata.ts Show resolved Hide resolved
apps/graph-api/src/metadata.ts Show resolved Hide resolved
@aramikm aramikm self-requested a review September 11, 2024 19:21
@JoeCap08055 JoeCap08055 force-pushed the chore/refactor-const-enums-interfaces-to-common-component branch from 01b096f to 61a1ef6 Compare September 12, 2024 00:51
@JoeCap08055 JoeCap08055 force-pushed the chore/refactor-const-enums-interfaces-to-common-component branch from bc1b86e to 903f438 Compare September 16, 2024 18:05
@JoeCap08055 JoeCap08055 merged commit 6f35e95 into main Sep 17, 2024
13 checks passed
@JoeCap08055 JoeCap08055 deleted the chore/refactor-const-enums-interfaces-to-common-component branch September 17, 2024 16:41
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.

Refactor constants, enums, interfaces & types to a common shared component
2 participants