Thanks Cris,
while you bring up valid concerns, this release is not "worse" than the
previous one. I've increased the overall code coverage - still not
optimal. Especially the new code is covered. And in fact it's fixing a
bug introduced in 1.5.0.
Based on that I don't see a technical reason to discard the vote. All
the points can be addressed in a follow-up release.
Regards
Carsten
Am 19.04.2021 um 15:23 schrieb Cris Rockwell:
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
--
--
Carsten Ziegeler
Adobe Research Switzerland
[email protected]
