Watson
Watson copied to clipboard
WIP: Add log messages to frames (2)
In my opinion, there is no need for PR #115 to be blocking the implementation of this feature.
So as a first step, I've taken the work done in PR #119 and made it independent of PR #115. The work done in PR #115 can be completed at a later time.
- [x] Fix the tests
- [ ] Test and check that everything works as expected with the
CLI
(and maybe add some tests?) - [ ] Add support for log messages in
log
andreport
command - [ ] Add documentation
Please note that I do not use watson with the CLI
at all and mainly use watson through the Watson
and Frame
classes. So any help and review to improve this part of the implementation is very welcomed.
@jmaupetit @jnsebgosselin For the last year this feature has become stalled a few times and is having trouble getting merged. In light of that, what do you think about merging this as-is? It looks good to me.
The critical parts are here: basic tests, documentation, and the parameter on stop. People can follow up with separate PRs for the other features, like enhancing log and report.
I feel like merging this basic functionality will help us gain momentum on this feature, as opposed to waiting for one giant pull request that addresses everything (perfect tests, complete documentation, all commands enhanced to support messages).
I do not agree with this strategy. If @jnsebgosselin agrees (and you have time & motivation for this), why not taking the lead on this?
I'm 100% OK if someone want to take the lead with this!
The thing is, I do not use the CLI at all. I use Watson inside a GUI I've made, in which I expanded Watson with the features proposed here. So the motivation to work on the CLI and its documentation is not very high for me.
@jnsebgosselin Ok. I will fork this draft and add support for the report and log commands. Thanks for letting me use your initial work as a starting point.
I'm not sure how soon I'll be able to find the time, but this is highest on list my of enhancements I'd like to see to watson.
@jmaupetit I've copied @jnsebgosselin 's branch and duplicated this PR in #337 . More discussion and questions are over in issue #93