-
Notifications
You must be signed in to change notification settings - Fork 430
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 more & easier "Environment" to bug template, and other tweaks #682
Conversation
Code Climate has analyzed commit 46a5efb and detected 0 issues on this pull request. View more on Code Climate. |
On my machine, the code snippet gives output like: Operating system:
Package versions:
|
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.
I like the idea of making it easier to capture diagnostic information from users (to be seen whether it gets uptake!). The script worked for me on Mac and Xubuntu Linux.
.github/ISSUE_TEMPLATE/bug_report.md
Outdated
packages=subprocess.run(["pip", "freeze"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT).stdout.decode("utf-8") | ||
print("""Operating system: `{}` | ||
Python version: | ||
``` |
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.
I think that the multiline string constant here could be compressed substantially into fewer lines to make this more compact?
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.
I think that the multiline string constant here could be compressed substantially into fewer lines to make this more compact?
I considered this, but I thought that users may appreciate being able to understand the code more easily, since we're asking them to run code on their machine... however, now that I think about it, a user is presumably already running our code (in the form of the library itself), so it's probably fine. I've switched to a more compact form, and added a suggestion that it can be run in a jupyter notebook too.
also should we consider something that will run on python2 (just-in-case?!)
Good idea, I've switched to using Popen
in a way that seems to work on Python 2 and 3.
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.
I had second thoughts about the python2 support; would need an additional from __future__ import print_function
which is probably a waste
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.
I think it's ok, because it's a single argument the parens are just grouping (i.e. (x)
is the same as x
).
.github/ISSUE_TEMPLATE/bug_report.md
Outdated
packages=subprocess.run(["pip", "freeze"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT).stdout.decode("utf-8") | ||
print("""Operating system: `{}` | ||
Python version: | ||
``` |
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.
I had second thoughts about the python2 support; would need an additional from __future__ import print_function
which is probably a waste
This tweaks the issue template in a few ways:
This is prompted by noticing some bugs that don't have as much info as would be helpful, so hopefully this serves as somewhat of a simplification.