rm5248 commented on a change in pull request #90:
URL: https://github.com/apache/logging-log4cxx/pull/90#discussion_r776893142
##########
File path: src/main/include/log4cxx/logger.h
##########
@@ -617,13 +617,13 @@ class LOG4CXX_EXPORT Logger :
@throws RuntimeException if all levels are null in the hierarchy
*/
- virtual const LevelPtr getEffectiveLevel() const;
+ virtual const LevelPtr& getEffectiveLevel() const;
/**
Return the the LoggerRepository where this
<code>Logger</code> is attached.
*/
- log4cxx::spi::LoggerRepositoryWeakPtr getLoggerRepository()
const;
+ log4cxx::spi::LoggerRepository* getLoggerRepository() const;
Review comment:
After thinking about this for a few days, would it be better to lave
this as `LoggerRepositoryWeakPtr`? The signature of this method did change
about a year ago(so [some people have started fixing their code to use
it](https://issues.apache.org/jira/browse/LOGCXX-544?focusedCommentId=17462427&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17462427)),
and I would hate to ping-pong back and forth over the signature of the method.
If we just change the `Logger::setHierarchy` to lock the pointer that is
passed in(and store the raw pointer as a new member), I think that would be
less of a change and less likely to break things.
I don't have a problem changing it in the next_stable branch, since my hope
would be that we can clean up the initialization sequence of things to make it
more clean as well.
Thoughts?
--
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]
