You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@struts.apache.org by "lukaszlenart (via GitHub)" <gi...@apache.org> on 2025/01/05 16:45:47 UTC

[PR] WW-5512 Extends the container to support injecting optional parameters into constructor [struts]

lukaszlenart opened a new pull request, #1175:
URL: https://github.com/apache/struts/pull/1175

   WW-5512


-- 
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: notifications-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] WW-5512 Extends the container to support injecting optional parameters into constructor [struts]

Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on PR #1175:
URL: https://github.com/apache/struts/pull/1175#issuecomment-2599642327

   How about the following - both parameters are injected as `null` even though one exists. Is that expected? I guess my concern here is that unintuitive behaviour here could lead to bugs being introduced in the future
   ```
   @Inject(required = false)
   public ConstructorCheck(@Inject(value = "constructorCheck.name") String name, @Inject(value = "nonExistingConstant") String name2) {
   ```


-- 
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: notifications-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] WW-5512 Extends the container to support injecting optional parameters into constructor [struts]

Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart merged PR #1175:
URL: https://github.com/apache/struts/pull/1175


-- 
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: notifications-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] WW-5512 Extends the container to support injecting optional parameters into constructor [struts]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #1175:
URL: https://github.com/apache/struts/pull/1175#issuecomment-2571687767

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1175) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [22 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=1175&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [42.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1175&metric=new_coverage&view=list) (required ≥ 80%)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [3.4% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=1175&metric=new_duplicated_lines_density&view=list) (required ≤ 3%)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [E Security Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1175) (required ≥ A)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [E Reliability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1175) (required ≥ A)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=1175)
   
   ##   
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png '') Catch issues before they fail your Quality Gate with our IDE extension ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png '') [SonarQube for IDE](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
   
   


-- 
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: notifications-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] WW-5512 Extends the container to support injecting optional parameters into constructor [struts]

Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on PR #1175:
URL: https://github.com/apache/struts/pull/1175#issuecomment-2599643714

   I can add a test case for that, but that should block such a situation
   
    ```(constructor.getParameterCount() > 0 && parameterInjectors == null)```
   
   constructor has parameters defined but there is nothing to inject.


-- 
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: notifications-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] WW-5512 Extends the container to support injecting optional parameters into constructor [struts]

Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on PR #1175:
URL: https://github.com/apache/struts/pull/1175#issuecomment-2599475617

   Does this also work for constructors with multiple optional parameters where 1 is available and the other is not?


-- 
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: notifications-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] WW-5512 Extends the container to support injecting optional parameters into constructor [struts]

Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on PR #1175:
URL: https://github.com/apache/struts/pull/1175#issuecomment-2599633052

   This is strictly for missing all the parameters and no default constructor exists.


-- 
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: notifications-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] WW-5512 Extends the container to support injecting optional parameters into constructor [struts]

Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on PR #1175:
URL: https://github.com/apache/struts/pull/1175#issuecomment-2599646616

   Yep if you can add a test that verifies the behaviour for my given example hasn't changed (injection exception is thrown) that would make me more comfortable


-- 
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: notifications-unsubscribe@struts.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org