Hi,

-1 (non-binding) for now

All the work happened directly on the master branch. Why not have a PR so 
others can review the changes in their totality?
PR's make it easier to review, because you get a clear picture of the overall 
changes and test coverage[0].

Issue 8825 is not included in this release. But it seems like a valid issue. 
String comparison with == just compares the instance identifiers,
so I don’t get how that would ever result true
https://issues.apache.org/jira/browse/SLING-8825 
<https://issues.apache.org/jira/browse/SLING-8825> 
https://github.com/apache/sling-org-apache-sling-auth-core/pull/1 
<https://github.com/apache/sling-org-apache-sling-auth-core/pull/1>


New code does not quite add up to 80% coverage. 
https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-auth-core&metric=new_coverage&view=list

Since SlingAuthenticator.java handleSecurity and doHandleSecurity methods has 
multiple uncovered branches. 
As stated in the issue, this bundle handles some core security concerns. 

There should be a way to get these covered.
https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-auth-core&metric=new_coverage&selected=apache_sling-org-apache-sling-auth-core%3Asrc%2Fmain%2Fjava%2Forg%2Fapache%2Fsling%2Fauth%2Fcore%2Fimpl%2FSlingAuthenticator.java&view=list
 
<https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-auth-core&metric=new_coverage&selected=apache_sling-org-apache-sling-auth-core%3Asrc%2Fmain%2Fjava%2Forg%2Fapache%2Fsling%2Fauth%2Fcore%2Fimpl%2FSlingAuthenticator.java&view=list>


Regards
Cris 



> On Apr 19, 2021, at 3:24 AM, Robert Munteanu <[email protected]> wrote:
> 
> On Sun, 2021-04-18 at 16:56 +0200, Carsten Ziegeler wrote:
>> Please vote to approve this release:
> 
> +1
> Robert

Reply via email to