Revamp the LogService #272
Conversation
|
❤️ You simplified a lot of stuff 👍 The last four commits regarding RootLogger are a bit confusing. |
Well, the current state of the branch is a huge mess. It does not merge cleanly, and there are a bunch of WIPs. In short, the branch still needs a substantial amount of work before it can be merged, and I do not have time to work on it myself. If you guys want it merged before Konstanz, you'll have to present a clean, mergeable branch with good unit test coverage. If you are able to do that before Monday, September 11, I'll make time to review it then before the hackathon, with the goal of getting it merged to master that week. It would be ideal if you were available that week for at least one more iteration of review and discussion, since I expect we'll still have some kinks to iron out. |
|
I still have an idea how to eliminate the strange |
maarzt
force-pushed
the
log-revamp
branch
2 times, most recently
from
6237d29 to
8d127f7
Compare
|
Hi @ctrueden, the branch is ready to be reviewed. I rewrote the history to get rid of the WIP commits. Now it should be nice an clean. No changes in it that are later reverted and so one. And I god rid of the package-private |
|
I added many tests to ensure, that there are no API changes breaking client code. see 93dffbd I available the whole week for discussions, or if I should change something. @ctrueden hope you're not too busy and we can work on this, this week. |
|
Thanks for your efforts, @maarzt! I am out sick today, but will try to follow up on this as soon as I feel better. |
|
I started looking through the commits yesterday, and got about halfway through. Will continue today. |
|
Nice! Is there already something you which to discuss? |
Not yet. I am looking at cherry-picking some of the standalone improvements you made onto master right away, to cut down on this monster. I rebased over latest master, will rebase again to fix very small issues (e.g. typos in commit messages), and force push that later today. For substantive changes etc., I have some commits I've done on top of the branch, which I will also push. I think the sanest plan would be for you to look over my new commits between now and next Wednesday, and then we can merge it together on Wednesday at the hackathon. I'll comment here again after I do the pushes. |
|
What's the current status, guess you got stuck somewhere? Maybe I can help cutting the monster? Tell me which commits you consider as standalone changes. I can then put them to the beginning of this branch, to make them easier to be merged separately. |
|
Sorry, I ran out of time. I will try to work on it again today, and/or on
my flight, but it might have to wait till Konstanz.
|
And test that the method works as expected. Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
ctrueden
force-pushed
the
log-revamp
branch
from
8d127f7 to
30b4295
Compare
|
13 of the commits are now merged to master. But there are still 23 to go. |
ctrueden
force-pushed
the
log-revamp
branch
3 times, most recently
from
26e9fbe to
8e5e17f
Compare
|
I am now done massaging the commits. I think it is looking pretty good. There are three commits I still want to discuss, which I moved to a separate branch @maarzt Please take a look and let me know what you think. We can hash out remaining issues in person when you arrive at the hackathon. |
This provides an implementation of Logger that can be used independently from AbstractLogService. It will be useful for the implementation of sub loggers.
maarzt
force-pushed
the
log-revamp
branch
2 times, most recently
from
cea4622 to
a22902b
Compare
|
I did a force push because: There where some commits fixing other commits. I squashed them, so the dirty code never appears in the history. I sorted the commits, by the changes made:
|
|
@maarzt Great. All the commits compile with passing tests. If you have no objections, I will merge now. We can discuss the |
It is a general-purpose interface for something that notifies listeners when a particular kind of event occurs.
These methods allow to create a Logger instance that is a child of another Logger instance. Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
And test it. Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
Now, the setPrintStreams method lets the caller define which output stream is used for each log level. This commit also adds a regression test verifying that the log is written to output streams as expected. Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
This PreprocessorPlugin affects modules with a single Parameter of type Logger. It will get a logger from LogService, named like the module's class, and assign it to the Parameter.
If label is specified in the @parameter annotation, the logger is given a name matching the provided label. Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>
The org.scijava.log package has new API.
When a type is Logged, it has a Logger, not necessarily a LogService. But for now, for backwards compatibility, we cannot change it yet.
We could extend the new Listenable interface. But it would break backwards compatibility, so for the moment, we don't.
ctrueden
force-pushed
the
log-revamp
branch
from
a22902b to
10f74fd
Compare
Continuation of #253 and #255.