Conversation
| if ext == '.json': | ||
| to_filetype = 'yaml' | ||
| elif 'yaml' in ext: | ||
| elif ext in ['.yaml', '.yml']: |
There was a problem hiding this comment.
This looks good!
Can we have a test for it? (e.g. in https://github.com/tmux-python/tmuxp/blob/master/tests/test_cli.py#L725-L733)
There was a problem hiding this comment.
@catroll at your request I can also merge this and I can write the test after merge. Whichever works best for you :)
There was a problem hiding this comment.
Should I write some test first?
But I'm not good enough at pytest, so maybe need some time to learn it.
There was a problem hiding this comment.
I will give you the choice, since it's an opportunity to learn. I can wait.
If you'd like to learn and write it yourself, the instructions here:
- https://tmuxp.git-pull.com/developing.html should let you set it up.
- pytest documentation
As an alternative, if that's too much effort for a fix of this size, please let me know if you'd like to skip writing a test this time.
There was a problem hiding this comment.
OK, I will write test myself, after work.
There was a problem hiding this comment.
i write some tests
please review it
is it ok?
@tony
There was a problem hiding this comment.
I ran the tests, I will take a look evening time (by me)
P.S. To avoid merge commits, you can do real rebase, e.g.
In your branch:
git remote add tmux-python https://github.com/tmux-python/tmuxp.git
git pull --rebase tmux-python master
git push --force-with-lease
|
|
tmuxp/cli.py
Outdated
| elif ext in ['.yaml', '.yml']: | ||
| to_filetype = 'json' | ||
| else: | ||
| raise click.BadParameter('Unknown filetype: %s (valid: [.json, .yaml, .yml])' % (ext, )) |
There was a problem hiding this comment.
It may be helpful to run black here
in re: ./tmuxp/cli.py:1195:89: E501 line too long (96 > 88 characters)
We've recently added a precommit bot so this should get easier
Co-authored-by: Tony Narlock <tony@git-pull.com>
Codecov Report
@@ Coverage Diff @@
## master #725 +/- ##
==========================================
+ Coverage 76.14% 76.26% +0.12%
==========================================
Files 7 7
Lines 1161 1163 +2
Branches 301 301
==========================================
+ Hits 884 887 +3
Misses 196 196
+ Partials 81 80 -1
Continue to review full report at Codecov.
|
|
@catroll Well done! Merged. This will be part of the next release |
No description provided.