-
Notifications
You must be signed in to change notification settings - Fork 136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pyxform version to XForm output, closes #393 #394
Conversation
Oops. Will try to fix all those failing tests. |
Don't worry about it. None of us know what we are doing and yet here we are... |
YEAH! @MartijnR out here writing Python!! I was hoping this would be really straightforward and get you excited about more pyxform contributions 😉 but hadn't anticipated the test problem. It seems it might be tricky to address because the old style tests do literal comparisons against expected XForm output and the version is dynamic. Darn. I wonder if there's a way to exclude that attribute in the diff? Or maybe it's time to switch those tests over to the new style? |
Haha, I was excited initially, indeed!
Exactly what I have finally concluded! Converting to new style tests would seem great. I am trying to remove the attribute from |
Codecov Report
@@ Coverage Diff @@
## master #394 +/- ##
==========================================
- Coverage 82.58% 82.56% -0.02%
==========================================
Files 23 23
Lines 3364 3373 +9
Branches 781 782 +1
==========================================
+ Hits 2778 2785 +7
- Misses 440 442 +2
Partials 146 146
Continue to review full report at Codecov.
|
dddc5b0
to
bbeb92c
Compare
bbeb92c
to
5bf9228
Compare
Thank you! That saves me from having to fix my python environment which would have probably taken many hours. The space is fine with me. |
pyxform/xls2json.py
Outdated
@@ -1395,6 +1397,12 @@ def get_parameters(raw_parameters, allowed_parameters): | |||
return params | |||
|
|||
|
|||
def get_git_describe_tags(): | |||
return subprocess.check_output( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be wrapped in a try/except for the unlikely case that git is not installed on the host machine. I think that in that case we should return something like unknown pyxform version
or simply pyxform
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error checking request below but otherwise this looks ready to me.
53e8705
to
5b303b9
Compare
5b303b9
to
46ae43c
Compare
46ae43c
to
8caae06
Compare
Remove support for generated-by reverting #394
Please check extra carefully, as I didn't really know what I was doing.
This code assumes each tag has the format 'v'+semantic version, as we've been using so far (except for vv0.88). If you think that may possibly change, I'd be happy to add a few lines to chop off any non numeric starting characters. Or we could not omit any characters (except trimming) and leave it up the data collection clients to do.