fix(#34) Add support for logging unix Timestamp #35
Conversation
|
okey I tested this, works - but I would love to see a unit test on it. |
|
Can someone add some directions on how to run tests etc ?. Hmm. got it from the travis runs. |
astrofrog
left a comment
astrofrog
left a comment
There was a problem hiding this comment.
Thanks for the pull request! Just a comment below about the naming of the argument and the behavior.
For tests, take a look at e.g. this test:
https://github.com/astrofrog/psrecord/blob/master/psrecord/tests/test_main.py#L51
To run tests locally, install the pytest package and do:
pytest psrecord
| 'in a slower maximum sampling rate).', | ||
| action='store_true') | ||
|
|
||
| parser.add_argument('--use-unix-ts', |
There was a problem hiding this comment.
Even though it would be a little more verbose, I think I'd prefer --include-timestamp and make it so that it simply adds a column (e.g. at the start) rather than replace the relative time.
|
Would you be opposed to just including the timestamp by default as well as the elapsed time? Both have their value and it's not really any more work to include them both. |
3 participants
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Testing procedure
Case 1
--use-unix-tswhile running psrecrodReview and Merge
Who
Changelog
Note