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

[ELY-2465] JsonSerialization#createObjectNode should throw its exception using ElytronMessages #1831

Merged
merged 1 commit into from
Nov 1, 2022
Merged

[ELY-2465] JsonSerialization#createObjectNode should throw its exception using ElytronMessages #1831

merged 1 commit into from
Nov 1, 2022

Conversation

lvydra
Copy link
Contributor

@lvydra lvydra commented Oct 31, 2022

cam-rod
cam-rod suggested changes Oct 31, 2022
View reviewed changes
Copy link
Contributor

@cam-rod cam-rod left a comment

Choose a reason for hiding this comment

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

Hey, looks good! Although it's a bit inconsistent, generally we prefer to keep the ElytronMessages class in a _private folder (example). Could you move the class into a matching folder and update the interface to be public?

@lvydra
Copy link
Contributor Author

lvydra commented Oct 31, 2022

Hi @cam-rod, thanks for the review, PR should be updated now.

fjuma
fjuma approved these changes Oct 31, 2022
View reviewed changes
Copy link
Contributor

@fjuma fjuma left a comment

Choose a reason for hiding this comment

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

Thanks @lvydra!

@cam-rod
Copy link
Contributor

cam-rod commented Oct 31, 2022

Looks good, thanks @lvydra!

@Skyllarr Skyllarr merged commit d8b1a66 into wildfly-security:1.x Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cam-rod cam-rod cam-rod requested changes

@fjuma fjuma fjuma approved these changes

@Skyllarr Skyllarr Skyllarr approved these changes

Assignees
No one assigned
Labels
hacktoberfest-accepted
Projects
None yet
Milestone
No milestone
4 participants
@lvydra @cam-rod @fjuma @Skyllarr