Skip to content

Identity-3304 Implement SCIM PATCH operation for Users#15

Merged
hpmtissera merged 3 commits into
wso2:masterfrom
cdwijayarathna:IDENTITY-3304
Jun 23, 2015
Merged

Identity-3304 Implement SCIM PATCH operation for Users#15
hpmtissera merged 3 commits into
wso2:masterfrom
cdwijayarathna:IDENTITY-3304

Conversation

@cdwijayarathna
Copy link
Copy Markdown
Contributor

No description provided.

@cdwijayarathna
Copy link
Copy Markdown
Contributor Author

This PR depends on wso2-attic/carbon-identity#442

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the comment, error should be logged. Also it should throw internal server error instead of ResourceNotFoundException according to the comment. Isn't it ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think since this is issue regarding no resource find for given information, resourceNotFoundException would be ideal, WDYT?

@cdwijayarathna
Copy link
Copy Markdown
Contributor Author

@thariyarox Thanks for reviweing, added suggested improvements.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

run sonar and Findbugs

@cdwijayarathna cdwijayarathna changed the title Identity-3304 Identity-3304 Implement SCIM PATCH operation for Users Jun 18, 2015
hpmtissera added a commit that referenced this pull request Jun 23, 2015
Identity-3304 Implement SCIM PATCH operation for Users
@hpmtissera hpmtissera merged commit 8fb2262 into wso2:master Jun 23, 2015
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can be appended to previous line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed with #39

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.

5 participants