lukaszlenart commented on PR #535:
URL: https://github.com/apache/struts-examples/pull/535#issuecomment-5292943130

   Thanks for this — the `ProduceActionTest` coverage and the `ProduceAction` 
cleanup are welcome, and this module had no tests at all before.
   
   Heads up that #544 just landed and overlaps part of this PR, which is why it 
now shows conflicts. On current `main`, `json-customize` already has:
   
   - `web.xml` migrated to the Jakarta EE 6.0 schema (same target as your 
change)
   - `struts.xml` on `https://struts.apache.org/dtds/struts-6.5.dtd`
   - the POM schema URL moved to `https://`
   
   So when you rebase, the XML parts of this PR can simply be dropped. Two of 
them would otherwise move things backwards:
   
   - your `struts.xml` points at 
`http://struts.apache.org/dtds/struts-6.0.dtd`, where `main` is now on 
`https://` and `6.5`
   - your `web.xml` keeps `id="struts_blank"`/`id="WebApp_ID"`, which the 
migration dropped repo-wide
   
   The valuable parts — `ProduceActionTest.java`, and the changes to 
`ProduceAction`, `ExcludeTransformer`, and `FlexJSONWriter` — rebase cleanly as 
far as I can tell.
   
   One thing to check while you're rebasing: 
`struts.parameters.requireAnnotations` has defaulted to `true` since before 
7.2.1, so any action property that receives a request parameter needs 
`@StrutsParameter` (depth 0 on the setter, depth ≥ 1 on the getter). #544 fixed 
seven modules that were silently dropping parameters. If `ProduceAction` takes 
any request input, it likely needs the annotation too — and a test that posts a 
parameter and asserts it arrives would catch exactly that class of bug.
   
   CI is also currently red here (`build (21)` failed, `build (17)` cancelled) 
from before the conflicts, so worth a look after the rebase.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to