Starts logging #34
Starts logging #34
Conversation
README.md
Outdated
| @@ -66,6 +66,8 @@ checksums of files in the latest version, while the command in (4) | |||
| behavior. For example, to update the checksums while checking the | |||
| diff, run `mvn starts:diff -DupdateDiffChecksums=true`. | |||
|
|
|||
| [Logging and Artifact storage Docs](./STARTS-LOGGING.md) | |||
STARTS-LOGGING.md
Outdated
| For any piece of starts that you'd like to add logging to, begin by adding two import statements at the top: | ||
| - ``import edu.illinois.starts.util.Logger;`` | ||
| - ``import java.util.logging.Level;`` | ||
| _note: you may need to make minor changes to the positioning of these two import statements to ensure checkstyle does not break. place ``edu.illinois.starts.util.Logger`` directly underneath the other imports with the same package name, and do the same with ``java.util.logging.Level``. additionally, ensure you have a newline separating different package names_ |
There was a problem hiding this comment.
Capitalize place, capitalize additionally. Add period at the end (after "different package names").
Is there a way to get this note to appear underneath the previous point (import java.util.logging.Level;), indented along with it, but without the bullet point? If possible, I think it would look nicer.
STARTS-LOGGING.md
Outdated
| - FINEST (lowest value) | ||
|
|
||
| To set the logging level of your log, use the ``setLoggingLevel(Level level)`` method. | ||
| i.e. |
There was a problem hiding this comment.
i.e. -> i.e.,
Can the code go on a separate line?
STARTS-LOGGING.md
Outdated
| ``logger.setLoggingLevel(Level.CONFIG);`` | ||
|
|
||
| To check the logging level, use the ``getLoggingLevel()`` method, which will return an object of type [Level](https://docs.oracle.com/javase/8/docs/api/java/util/logging/Level.html). | ||
| i.e. |
There was a problem hiding this comment.
i.e. -> i.e.,
Can the code go on a separate line (not inlined)?
STARTS-LOGGING.md
Outdated
| ###### ``public void log(Level lev, String msg)`` | ||
| should be used when you only want to have a custom message | ||
|
|
||
| i.e. ``logger.log(Level.SEVERE, "houston we have a problem");`` |
STARTS-LOGGING.md
Outdated
| ## Where will messages be output? | ||
| Standard Output (System.out) | ||
|
|
||
| ## Artifact Storage |
There was a problem hiding this comment.
Uncapitalize Storage (for consistency of the other sections).
STARTS-LOGGING.md
Outdated
|
|
||
| ``mvn starts:starts -DStartsLogging=<Level>`` | ||
|
|
||
| i.e. ``mvn starts:starts -DStartsLogging=FINEST`` |
STARTS-LOGGING.md
Outdated
| - dependency file (.starts/deps.zlc) | ||
|
|
||
| Level.FINER will store: | ||
| - dependency file (.starts/deps.zlc) |
There was a problem hiding this comment.
Shouldn't FINER include all the things in INFO, including checksum (actually, isn't checksum a part of dependency file)?
STARTS-LOGGING.md
Outdated
| - list of impacted tests | ||
|
|
||
| Level.FINEST will store: | ||
| - list of all tests |
There was a problem hiding this comment.
Similarly, shouldn't FINEST also contain of of FINER (currently missing dependency file/checksum)?
STARTS-LOGGING.md
Outdated
|
|
||
| To set the log level at runtime, call starts like this: | ||
|
|
||
| ``mvn starts:starts -DStartsLogging=<Level>`` |
There was a problem hiding this comment.
I believe the option actually has it lower case, i.e., startsLogging. Please double check.
|
@august782 thanks for the feedback. I've rebased to include the two merges that Owolabi recently made to testingresearchillinois/starts, and added the fixes you've suggested |
|
@cptwonton, please rebase master, into this branch, and we can see if it is ready to merge. |
|
@owolabileg this is done |
cptwonton
force-pushed
the
STARTS-LOGGING
branch
from
1a3e444
to
eaa0f1a
Compare
adds logging documentation to STARTS, incorporating the paper's details on artifact storage
cptwonton
force-pushed
the
STARTS-LOGGING
branch
2 times, most recently
from
0805871
to
e144cb2
Compare
|
@owolabileg rebased and squashed and ready for merging into master |
|
Closes #4 |
There was a problem hiding this comment.
Has there been any discussion on whether or not this documentation should live in another place, like a docs/ folder (or even the Github wiki)?
@owolabileg other than my comments, this pr LGTM.
Co-authored-by: Benjamin Shen <[email protected]>
Co-authored-by: Benjamin Shen <[email protected]>
adds documentation for logging functionality in STARTS
includes info about artifact storage from paper
adds link in README.md to starts logging docs