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 AccountResource, UserResource and EntityResource to use Problem #6411

Merged
merged 1 commit into from Oct 5, 2017

Conversation

Projects
None yet
3 participants
@cbornet
Member

cbornet commented Sep 26, 2017

  • Please make sure the below checklist is followed for Pull Requests.

  • Travis tests are green

  • Tests are added where necessary

  • Documentation is added/updated where necessary

  • Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

Throw Problem exceptions that will be treated by the ExceptionTranslator instead of returning ResponseEntity with an unchecked body
See #6404

@jdubois

This comment has been minimized.

Show comment
Hide comment
@jdubois

jdubois Sep 27, 2017

Member

LGTM - as this is quite big, does anybody has any comment on this, before merging it?

Member

jdubois commented Sep 27, 2017

LGTM - as this is quite big, does anybody has any comment on this, before merging it?

@mraible

This comment has been minimized.

Show comment
Hide comment
@mraible

mraible Sep 27, 2017

Contributor

I'd love to get #6361 in first since this touches AccountResource and UserResource quite a bit too. I think it's close to complete.

Contributor

mraible commented Sep 27, 2017

I'd love to get #6361 in first since this touches AccountResource and UserResource quite a bit too. I think it's close to complete.

@jdubois

This comment has been minimized.

Show comment
Hide comment
@jdubois

jdubois Sep 27, 2017

Member

You are right @mraible - your PR is top priority as it's really a huge work

Member

jdubois commented Sep 27, 2017

You are right @mraible - your PR is top priority as it's really a huge work

@jdubois

This comment has been minimized.

Show comment
Hide comment
@jdubois

jdubois Oct 2, 2017

Member

@cbornet so of course we broke your code, but I don't think it's that bad -> WDYT?

Member

jdubois commented Oct 2, 2017

@cbornet so of course we broke your code, but I don't think it's that bad -> WDYT?

@cbornet

This comment has been minimized.

Show comment
Hide comment
@cbornet

cbornet Oct 2, 2017

Member

I'll have a look. Shouldn't be too hard

Member

cbornet commented Oct 2, 2017

I'll have a look. Shouldn't be too hard

@cbornet

This comment has been minimized.

Show comment
Hide comment
@cbornet

cbornet Oct 2, 2017

Member

Rebased

Member

cbornet commented Oct 2, 2017

Rebased

@cbornet cbornet added the needs-review label Oct 2, 2017

@jdubois

This comment has been minimized.

Show comment
Hide comment
@jdubois

jdubois Oct 3, 2017

Member

You still have a conflict :-P

Member

jdubois commented Oct 3, 2017

You still have a conflict :-P

@cbornet

This comment has been minimized.

Show comment
Hide comment
@cbornet

cbornet Oct 3, 2017

Member

Yeah, that happens when pushing muliple related PRs 😄 . Not a big deal.

Member

cbornet commented Oct 3, 2017

Yeah, that happens when pushing muliple related PRs 😄 . Not a big deal.

Refactor AccountResource, UserResource and EntityResource to use Problem
Throw exceptions that will be treated by the ExceptionTranslator instead of returning ResponseEntity with an unchecked body
See #6404
@cbornet

This comment has been minimized.

Show comment
Hide comment
@cbornet

cbornet Oct 4, 2017

Member

Rebased (will conflict with #6472 😄 )

Member

cbornet commented Oct 4, 2017

Rebased (will conflict with #6472 😄 )

@cbornet cbornet removed the needs-review label Oct 5, 2017

@cbornet cbornet merged commit cbbd1c0 into jhipster:master Oct 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@cbornet cbornet deleted the cbornet:account-problem branch Oct 5, 2017

@cbornet cbornet referenced this pull request Oct 5, 2017

Closed

Use Zalando Problem for all json responses #6404

1 of 1 task complete

@jdubois jdubois added this to the 4.10.0 milestone Oct 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment