Conversation
squito
left a comment
There was a problem hiding this comment.
Thanks so much for working on this @Beyondeclipse , I've wanted to add proper logging support!
I'm a little rusty on all the different logging libraries -- any particular reason you are using logback? I would have expected you to just use log4j as that is already loaded by spark in the jvm, and you'd reuse the same configuration for the rest of spark. If there is a good reason to use logback instead, can you give an example of how you'd set up logback configuration?
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| xmlns="http://maven.apache.org/POM/4.0.0" |
There was a problem hiding this comment.
I'd really prefer to stick w/ 2 space indent for this file
|
enn, I think it may be more suitable to use 'log4j' in this project for simplified and compatible reason. I used 'logback' before is just because my project is using it. In fact, I prefer 'log4j2' however spark is not using it. |
squito
left a comment
There was a problem hiding this comment.
thanks @Beyondeclipse . there aren't any real tests to speak of in this project yet, so this will have to wait till I get a chance to try it out, but looks good except for some really minor stuff.
| log4j.appender.file.file=target/unit-tests.log | ||
| log4j.appender.file.layout=org.apache.log4j.PatternLayout | ||
| log4j.appender.file.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss.SSS} %t %p %c{1}: %m%n | ||
| # Tests that launch java subprocesses can set the "test.appender" system property to |
There was a problem hiding this comment.
this is copied from spark, right? I think everything from here down is not relevant here (and so is confusing)
There was a problem hiding this comment.
Yes,I copy it for test only. So, do you mean to cut the 'console output' part?
| </plugin> | ||
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>build-helper-maven-plugin</artifactId> |
There was a problem hiding this comment.
mvn builds were working for me before ... why did you need to add this?
There was a problem hiding this comment.
This plugin is help to generate the source.jar by mvn. As I know there are many projects are mixture of java and scala in companies of China, especially the medium and minor ones. And I think it is convenient for them to get the source code from repository after using maven to deploy the project. So what is your opinion?
Add logback support