Watson icon indicating copy to clipboard operation
Watson copied to clipboard

WIP: Add log messages to frames (2)

Open jnsebgosselin opened this issue 5 years ago • 5 comments

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 and report 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.

jnsebgosselin avatar Feb 09 '19 03:02 jnsebgosselin

@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).

prat0088 avatar Nov 12 '19 17:11 prat0088

I do not agree with this strategy. If @jnsebgosselin agrees (and you have time & motivation for this), why not taking the lead on this?

jmaupetit avatar Nov 13 '19 08:11 jmaupetit

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 avatar Nov 13 '19 12:11 jnsebgosselin

@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.

prat0088 avatar Nov 13 '19 14:11 prat0088

@jmaupetit I've copied @jnsebgosselin 's branch and duplicated this PR in #337 . More discussion and questions are over in issue #93

prat0088 avatar Nov 17 '19 05:11 prat0088