-
Notifications
You must be signed in to change notification settings - Fork 65
Proposed minor tweaks #30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
senderic
commented
May 19, 2020
- Devise class implement AutoCloseable. This will allow usage of try-with-resources.
- Usage of slf4j-log4j12 and the creation of log4j.properties file that will pipe logs to both stdout and to a file.
- Devise class implement AutoCloseable. This will allow usage of try-with-resources. - Usage of slf4j-log4j12 and the creation of log4j.properties file that will pipe logs to both stdout and to a file.
- Do check, I don't think I have the developers filled out and the license may be off? BSD - Juniper will need to sign the jars. the maven-gpg-plugin can help automate that - See sample POM at bottom of this: https://central.sonatype.org/pages/requirements.html - Also, I see you already are publishing javadocs to files in the javadoc/ directory. I split them into different goals. - This cmd will produce the previous javadoc files `mvn javadoc:javadoc@javadoc`
Note: |
@@ -55,7 +55,7 @@ | |||
*/ | |||
@Slf4j | |||
@Getter | |||
public class Device { | |||
public class Device implements AutoCloseable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this small change is super awesome.
@@ -0,0 +1,16 @@ | |||
# Root logger option |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some people, they will appreciate default logging to a file., I could imagine some others would rather leave it to themselves to configure. Currently asking some peers for their opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to go with it is okay. Any sane person will configure the logger on their app. I've certainly appreciated having logs their to examine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I set up the properties file so that logs go to both stdout as well as a file. The top line is where the developer can change the directive:
log4j.rootLogger=DEBUG, file, stdout
<-- take out file
to ensure it doesn't go into a file.
But either way, yes run it by your peers. I really added it in there so that I could do some debugging on my end as I worked through your API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems fine to me to keep it there. Thanks again! The more people that use the library the more bugs we can find and fix. :)