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.

Sign up for GitHub

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

Jump to bottom

Update to Edit Card Packages #35

Merged
merged 3 commits into from
Nov 21, 2022
Merged

Update to Edit Card Packages #35

merged 3 commits into from
Nov 21, 2022

Conversation

AlbertYS
Copy link
Contributor

Changes to EditCardInputData such that it uses card id number rather than card itself, now accepts editing of options with overloaded constructors EditCardInputBoundary changes as required

Updated implementations for:
EditCardInteractor
EditCardInputBoundary
EditCardController

AlbertYS and others added 3 commits November 18, 2022 19:08
@AlbertYS
Update to Edit Card Packages
51ed39a 
Changes to EditCardInputData such that it uses card id number rather than card itself, now accepts editing of options with overloaded constructors
EditCardInputBoundary changes as required

Updated implementations for:
EditCardInteractor
EditCardInputBoundary
EditCardController
@AlbertYS
Merge branch 'main' into AlbertEditCard
ab4aba7
@AlbertYS
Minor fixes to interactor such that it prepares outputData
fd4f20f
derrickcho3715
derrickcho3715 previously approved these changes Nov 20, 2022
View reviewed changes
Copy link
Contributor

@derrickcho3715 derrickcho3715 left a comment

Choose a reason for hiding this comment

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

This looks good. All the proper documentation is there and it has all the necessary files including tests. One thing to note is that the EditCardInteractor is missing a call to EditCardOutputBoundary. Apart from that it's good work.

jakan9
jakan9 previously approved these changes Nov 20, 2022
View reviewed changes
@AlbertYS AlbertYS dismissed stale reviews from jakan9 and derrickcho3715 via fd4f20f November 20, 2022 21:54
aldob02
aldob02 approved these changes Nov 20, 2022
View reviewed changes
Copy link
Contributor

@aldob02 aldob02 left a comment

Choose a reason for hiding this comment

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

Looks good, only testing and javadocs are needed.

derrickcho3715
derrickcho3715 approved these changes Nov 21, 2022
View reviewed changes
Copy link
Contributor

@derrickcho3715 derrickcho3715 left a comment

Choose a reason for hiding this comment

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

This looks good. All the proper documentation is there and it has all the necessary files including tests. The issue with the interactor and output boundary was properly addressed.

@AndrewNQN AndrewNQN merged commit db50626 into main Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakan9 jakan9 jakan9 approved these changes

@aldob02 aldob02 aldob02 approved these changes

@derrickcho3715 derrickcho3715 derrickcho3715 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AlbertYS @jakan9 @aldob02 @derrickcho3715 @AndrewNQN