Skip to content

Conversation

Hutchpro1
Copy link

@Hutchpro1 Hutchpro1 commented Dec 14, 2022

…ed Profiles Repository

Description

Create a ProfilesController class with CRUD implementation and created ProfilesService Class and updated the ProfilesRepository Interface.

Fixes # (issue)

Loom Video

(Video Link)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have removed unnecessary comments/console logs from my code
  • I have made corresponding changes to the documentation if necessary (optional)
  • My changes generate no new warnings
  • I have checked my code and corrected any misspellings
  • No duplicate code left within changed files
  • Size of pull request kept to a minimum
  • Pull request description clearly describes changes made & motivations for said changes

@Hutchpro1 Hutchpro1 marked this pull request as ready for review December 15, 2022 00:12
tkozzer
tkozzer previously approved these changes Dec 15, 2022
Copy link
Contributor

@tkozzer tkozzer left a comment

Choose a reason for hiding this comment

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

In your ProfilesController class you may want to consider adding some exception handling if you get bad request data from the frontend. Otherwise I think this is a good start.

import java.util.Optional;

@RestController
@RequestMapping("/profile/")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is super minor, but I don't think you need the extra '/' at the end of "/profile" in the RequestMapping annotation.

deepalisethia
deepalisethia previously approved these changes Dec 15, 2022
Copy link

@deepalisethia deepalisethia left a comment

Choose a reason for hiding this comment

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

The code looks good to me!

@Hutchpro1 Hutchpro1 dismissed stale reviews from deepalisethia and tkozzer via 6152b25 December 15, 2022 21:28
Copy link
Contributor

@tkozzer tkozzer left a comment

Choose a reason for hiding this comment

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

The changes that you made look good!

Copy link
Contributor

@dpearson00 dpearson00 left a comment

Choose a reason for hiding this comment

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

Good work!

@Kubwayojc
Copy link

Pedro, you did great work on this and I agree with Tyler's suggestions.

@ashtilawat ashtilawat self-assigned this Dec 22, 2022
@ashtilawat
Copy link
Contributor

Will look at this PR tomorrow and assign points.

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.

6 participants