Report location where the value is empty in mobanfile #139
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #139 +/- ##
==========================================
+ Coverage 98.44% 98.46% +0.02%
==========================================
Files 39 39
Lines 1801 1825 +24
==========================================
+ Hits 1773 1797 +24
Misses 28 28
Continue to review full report at Codecov.
|
seeeturtle
force-pushed
the
weird-yaml
branch
from
4d607ab
to
249e759
Compare
CHANGELOG.rst
Outdated
| @@ -1,6 +1,15 @@ | |||
| Change log | |||
| ================================================================================ | |||
|
|
|||
| 0.3.5.1 - 02-12-2018 | |||
There was a problem hiding this comment.
0.3.5 has not been released yet. So, your change will go with 0.3.5
|
Please update change log. Otherwise, all is good to merge! |
seeeturtle
force-pushed
the
weird-yaml
branch
from
249e759
to
9806b55
Compare
There was a problem hiding this comment.
Please update the changelog by doing the following:
- Update the changelog by editing this file:
.moban.cd/changelog.yml - Then, run
make update, which will auto-generateCHANGELOG.rstbased on your changes in step 1
By doing it this way, when someone runs make update in the future, it won't overwrite your updates to the changelog. I also made this mistake on my first try, so reference my PR if needed :)
Feel free to update the docs if this wasn't clear enough!
| import moban.reporter as reporter | ||
| import moban.constants as constants | ||
| import moban.exceptions as exceptions | ||
| from ruamel.yaml import YAML |
There was a problem hiding this comment.
Just a little improvement here. Since ruamel.yaml is a third party dependency, it should be placed in between import errno and import moban.reporter import YAML. (ref)
There was a problem hiding this comment.
hmm... make format put the import at that position. which should i follow?
There was a problem hiding this comment.
.isort.cfg needs updating.
here is the isort docs: https://isort.readthedocs.io
|
Call 'make format' will change coding style to confirm existing ones. |
If empty value is found in mobanfile, moban will report the line number of that and the name of the file. Closes moremoban#37
seeeturtle
force-pushed
the
weird-yaml
branch
from
9806b55
to
d0540fd
Compare
|
https://github.com/moremoban/moban/blob/dev/.isort.cfg defines the import order. You may want to put ruemal.yaml at the end of known first party. |
|
And please mention the issue number in change log, which will be rendered by moban as a link to github. |
| @@ -5,6 +5,7 @@ releases: | |||
| - action: Updated | |||
| details: | |||
| - "`#37`: switch from pyyaml to ruamel.yaml" | |||
| - "moban will report line number where the value is empty and the name of mobanfile" | |||
There was a problem hiding this comment.
this line is essentially part of #37. in order to get line number, we switch from pyyaml to ruamel.yaml
If empty value is found in mobanfile, moban will report the
line number of that.
Closes #37
Before raising the PR, here is a check list:
5.1) can someone else understand your changes without your explanation?
6.1) do you have the feeling of achievement?