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.

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

WFLY-14559: Renamed and updated WFLY-14559_Add_four_new_RESTEasy_cont… #560

Closed
wants to merge 4 commits into from

Conversation

ronsigal
Copy link
Contributor

…ext_parameters.adoc

Copy link
Member

@jamezp jamezp left a comment

Choose a reason for hiding this comment

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

Just a few minor comments.

Comment on lines 37 to 41
* {issue-base-url}/RESTEASY-2280[RESTEASY-2280]
* {issue-base-url}/RESTEASY-2646[RESTEASY-2646]
* {issue-base-url}/RESTEASY-2782[RESTEASY-2782]
* {issue-base-url}/RESTEASY-2849[RESTEASY-2849]
* {issue-base-url}/RESTEASY-2866[RESTEASY-2866]
Copy link
Member

Choose a reason for hiding this comment

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

Minor nit, this should just be https://issues.redhat.com/browse. We migrated away from the issue-base-url for ease of copy/paste when reviewing.

Comment on lines 17 to 22
* resteasy.match.cache.enabled: [RESTEASY-2646]
* resteasy.match.cache.size: [RESTEASY-2646]
* resteasy.original.webapplicationexception.behavior: [RESTEASY-2782]
* resteasy.patch.filter.disabled: [RESTEASY-2280]
* resteasy.patch.filter.legacy: [RESTEASY-2849]
* resteasy.proxy.implement.all.interfaces: [RESTEASY-2866]
Copy link
Member

Choose a reason for hiding this comment

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

Should we drop the resteasy prefix from the model attribute names? I know most of them have that prefix now, but it's not strictly required and saves some typing for administators.

* resteasy.patch.filter.legacy: [RESTEASY-2849]
* resteasy.proxy.implement.all.interfaces: [RESTEASY-2866]

The wildfly/jaxrs module should be extended to expose these new parameters.
Copy link
Member

Choose a reason for hiding this comment

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

s/wildfly/WildFly for consistency with other references.


== Overview

Since the RESTEasy context parameters were first exposed in the wildfly/jaxrs management model
Copy link
Member

Choose a reason for hiding this comment

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

s/wildfly/WildFly


== Requirements

* Some simple modifications to the wildfly/jaxrs module will be necessary to add these new parameters.
Copy link
Member

Choose a reason for hiding this comment

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

s/wildfly/WildFly


* resteasy.original.webapplicationexception.behavior:

testsuite/integration-tests/src/test/java/org/jboss/resteasy/test/client/exception/ClientWebApplicationExceptionMicroProfileProxyTest.java
Copy link
Member

Choose a reason for hiding this comment

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

Should these be links? I'm not convinced they should be, but it seems like we could easily enough make them links to the source.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, except for the WildFly tests. I guess I could add those after WildFly is released.

@@ -0,0 +1,102 @@
= WFLY-14559 Add six new RESTEasy context parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the jekyll front matter (the categories stuff).


=== Issue:

* {issue-base-url}/EAP7-1655[EAP7-1655]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this is a related issue; this does not directly address EAP7-1655.

The wildfly/jaxrs module should be extended to expose these new parameters.

[Note. RESTEasy uses '.' as a separator in parameter names. WildFly/jaxrs uses '-'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the Stability section.

@ronsigal
Copy link
Contributor Author

I didn't realize there was new activity here. I'll get on it.

@ronsigal
Copy link
Contributor Author

I don't know how I closed this.

@ronsigal
Copy link
Contributor Author

Reopening

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.

3 participants