search

fnogatz / clocker

Command-line tool to track project hours
Other
420 stars 31 forks source link

No verbose mode for report command after lib extraction #45

Closed BeniRupp closed 5 years ago

BeniRupp commented 5 years ago

During the refactoring in #40 the verbose mode of the report command get lost. So we should add it again.

BeniRupp commented 5 years ago

I'am working on it.

@fnogatz should the report command stay in the CLI part of clocker or can it be moved to the lib? The latter would increase testability. 🙂

fnogatz commented 5 years ago

should the report command stay in the CLI part of clocker or can it be moved to the lib? The latter would increase testability.

The focus of #40 was to split the functionality in library and CLI. The first are all functions that can be used programmatically (i.e., with var clocker = require('clocker')). We have to ensure that they are stable, as users rely on them and our correct use of semver.

The CLI contains all things just for interaction with the end-user, i.e., input parsing and serialisation. Therefore the clocker report command (which is just a wrapper for clocker.data()) is located here. I would suggest to leave this separation into lib+CLI as it is. It allows us to modify the serialised representation (CLI) when needed.

Though it might be a good idea to test the provided CLI separately. So my suggestion is to leave all searialisation to the CLI, but add tests for them separately. That means in addition to the current lib tests with clocker.start(() => { clocker.data((data) => { t.equal(data, ...) }) }), we should add tests for the CLI in the form of clocker start; clocker data # test stdout. Maybe there's an npm module for this, which can be easily integrated into the current TAP infrastructure.

BeniRupp commented 5 years ago

Okay, great! Thanks for this insights. 👍

Though it might be a good idea to test the provided CLI separately.

I am trying exactly that for the report command and the verbose option at the moment. So I will go on there and we can discuss the soultion later on.