stefan-egli commented on a change in pull request #4:
URL:
https://github.com/apache/sling-org-apache-sling-discovery-commons/pull/4#discussion_r708147273
##########
File path:
src/main/java/org/apache/sling/discovery/commons/providers/base/ViewStateManagerImpl.java
##########
@@ -590,6 +595,46 @@ public void run() {
}
}
+ protected boolean equalsIgnoreSyncToken(BaseTopologyView newView) {
+ if (previousView==null) {
Review comment:
This is a bit convoluted and thus not very user friendly indeed. The use
of `lock` in this class is a result of early refactoring to make the class
reusable. That arguably makes it a bit awkward, and I would actually prefer if
it was done differently. Having said that, the way it is currently, is that the
`lock` object is shared between this class and the caller (that's where
awkwardness comes in). And `equalsIgnoreSyncToken` is considered a method which
only happens within a `lock.lock()`.
I'll add a note to this method's javadoc to make it a bit more explicit and
clear
--
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]
