Skip to content

Conversation

@andreasjansson
Copy link
Member

Signed-off-by: andreasjansson [email protected]

@andreasjansson andreasjansson requested a review from bfirsh May 14, 2021 22:55
@bfirsh
Copy link
Member

bfirsh commented May 15, 2021

Nice!

A few thoughts:

  • We should avoid adding single character flags without a lot of thought. The process (which we should document) should probably be add a long flag, then over time add short flags after we tease out usage. User interface also could do with improvement here but I will be tearing apart user interface so no problem.
  • Would it be too hard to add basic tests for this behavior? I'm wondering now we are in a "beta" phase, we should be adding tests for any behavior.

If tests are hard or you don't think it's important feel free to merge!

Signed-off-by: andreasjansson <[email protected]>
Signed-off-by: andreasjansson <[email protected]>
@andreasjansson
Copy link
Member Author

@bfirsh Added test and removed -l shorthand.

@andreasjansson andreasjansson merged commit 58fe4d8 into main May 17, 2021
@andreasjansson andreasjansson deleted the andreas/push-log branch May 17, 2021 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants