vy commented on code in PR #3467:
URL: https://github.com/apache/logging-log4j2/pull/3467#discussion_r1959551727


##########
log4j-mongodb4/src/main/java/org/apache/logging/log4j/mongodb4/MongoDb4Connection.java:
##########
@@ -67,21 +67,23 @@ public MongoDb4Connection(
             final ConnectionString connectionString,
             final MongoClient mongoClient,
             final MongoDatabase mongoDatabase,
+            final String collectionName,

Review Comment:
   > The build system that log4j uses is something that I have not been able to 
get to work on my machine. Even if I solve odd errors like 
distributionSha256Sum challenges and downgrade the java version, it still gets 
blocked by issues with some of the docker steps due to ARM architecture of my 
Mac, so running that command isn't currently possible.
   
   This is a very important problem that we need to tackle. To start with, 
assuming you locally have JDK 17 installed, please go through 
[BUILDING.adoc](/apache/logging-log4j2/blob/2.x/BUILDING.adoc) first, unless 
you've already done so. Does `./mvnw install -DskipTests` work? What about 
`./mvnw verify -pl :log4j-mongodb,:log4j-mongodb4`? If not, please share the 
associated error messages and the commit ID you're building from.
   
   > I understand your point about the constructor, but the constructor is only 
called from inside the build method. And with the builder pattern people should 
be using the build method to handle all of those pieces anyway and not directly 
invoking the constructor.
   
   Since the method/ctor and the class is public, and exported, it is a part of 
our binary API that we cannot break the compatibility with. There might exist 
3rd party integrations using these API endpoints. This is a major difference 
between `main` (targeting the next major release, i.e., `3.0.0`, hence we are 
allowed introduce binary backward incompatibilities) and `2.x` (targeting the 
next `2.x` release, where breaking changes are not allowed).
   
   > This approach is inline with previously requested changes in the `main` 
branch and moving these would just make the change bigger and make it a larger 
difference between 2.x and the 3.x approaches.
   
   Agreed – welcome to the world of Log4j maintainers. :sweat_smile:



-- 
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-unsubscr...@logging.apache.org

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

Reply via email to