-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Add checklists for Github issue report and pull requests #6870
Changes from 4 commits
0b8597e
777232c
498577a
abac344
4066ba7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,34 @@ | ||
Before submitting your pull request be sure that: | ||
<!--- Before submitting your pull request, ---> | ||
<!--- please complete as much as possible of the following checklist: ---> | ||
|
||
1. You haven't touched any file in the `spyder/defaults` directory. | ||
- There is *absolutely* no need to touch those files. | ||
- If you want to add new configuration options, please go to | ||
`spyder/config/main.py`. | ||
2. You haven't eliminated unnecessary blank lines or spaces during your work. | ||
That makes our reviewing work harder and it could introduce unnecessary | ||
conflicts with other pull requests. | ||
3. You haven't added new icons to Spyder. Please leave decisions about what | ||
icons to use to us :) | ||
### Pull Request Checklist | ||
|
||
---- | ||
* [ ] Read and followed this repo's [Contributing Guidelines](https://github.com/spyder-ide/spyder/blob/master/CONTRIBUTING.md) | ||
* [ ] Based your PR on the latest version of the correct branch (master or 3.x) | ||
* [ ] Followed [PEP8](https://www.python.org/dev/peps/pep-0008/) for code style | ||
* [ ] Ensured your pull request hasn't eliminated unrelated blank lines/spaces, | ||
modified the ``spyder/defaults`` directory, or added new icons/assets | ||
* [ ] Wrote at least one-line docstrings for any new functions | ||
* [ ] Added at least one unit test covering the changes, if at all possible | ||
* [ ] Described your changes and the motivation for them below | ||
* [ ] Noted what issue(s) this pull request resolves, creating one if needed | ||
* [ ] Included a screenshot, if this PR makes any visible changes to the UI | ||
|
||
*Note*: You can safely remove this text before submitting your work. | ||
|
||
## Description of Changes | ||
|
||
<!--- Describe what you've changed and why. ---> | ||
|
||
|
||
|
||
|
||
### Issue(s) Resolved | ||
|
||
<!--- Pull requests should typically resolve at least one—preferably only one— | ||
<!--- outstanding issue; create a new one if no relevant issue exists. | ||
<!--- List the issue(s) below, in the form "Fixes #1234" . One per line.---> | ||
|
||
Fixes # | ||
|
||
|
||
<!--- Thanks for your help making Spyder better for everyone! ---> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,7 +141,7 @@ | |
# Local utility imports | ||
#============================================================================== | ||
from spyder import (__version__, __project_url__, __forum_url__, | ||
__trouble_url__, get_versions) | ||
__trouble_url__, __trouble_url_short__, get_versions) | ||
from spyder.config.base import (get_conf_path, get_module_data_path, | ||
get_module_source_path, STDERR, DEBUG, | ||
debug_print, MAC_APP_NAME, get_home_dir, | ||
|
@@ -2409,7 +2409,7 @@ def render_issue(self, description='', traceback=''): | |
revision = versions['revision'] | ||
|
||
# Store and format the reminder message for the troubleshooting guide | ||
reminder_message = ( | ||
reminder_message_full = ( | ||
"<!--- **PLEASE READ:** Before submitting here, please carefully " | ||
"consult our *Troubleshooting Guide*: {0!s} and search the " | ||
"issues page for your error/problem, as most posted bugs are " | ||
|
@@ -2420,34 +2420,53 @@ def render_issue(self, description='', traceback=''): | |
"will be closed. Thanks! --->" | ||
).format(__trouble_url__) | ||
|
||
reminder_message_short = ( | ||
"<!--- PLEASE READ: Complete the following checklist. " | ||
"Issues without it may be closed. --->" | ||
) | ||
|
||
bug_checklist = ( | ||
"* [ ] Searched issues page for similar reports\n" | ||
"* [ ] Read and followed relevant sections of the" | ||
"[Troubleshooting Guide]({0!s})\n" | ||
"* [ ] Reproduced after updating (`conda update spyder`)\n" | ||
"* [ ] Tried basic troubleshooting\n" | ||
" * [ ] Restarted Spyder\n" | ||
" * [ ] Ran `spyder --reset`\n" | ||
" * [ ] Reinstalled latest Anaconda\n" | ||
"* [ ] Completed all applicable sections below\n" | ||
).format(__trouble_url_short__) | ||
|
||
# Make a description header in case no description is supplied | ||
if not description: | ||
description = "### What steps will reproduce the problem?" | ||
description = "### What steps reproduce the problem?" | ||
|
||
# Make error section from traceback | ||
# Make error section from traceback and add appropriate reminder header | ||
if traceback: | ||
error_section = ("### Traceback\n" | ||
"```python-traceback\n" | ||
"{}\n" | ||
"```".format(traceback)) | ||
reminder_message = reminder_message_full | ||
else: | ||
error_section = '' | ||
reminder_message = reminder_message_short + "\n\n" + bug_checklist | ||
issue_template = """\ | ||
{reminder_message} | ||
|
||
## Problem Description | ||
## Description | ||
|
||
{description} | ||
|
||
{error_section} | ||
|
||
## Package Versions | ||
## Versions | ||
|
||
* Spyder version: {spyder_version} {commit} | ||
* Python version: {python_version} | ||
* Qt version: {qt_version} | ||
* {qt_api_name} version: {qt_api_version} | ||
* Operating system: {os_name} {os_version} | ||
* Spyder ver: {spyder_version} {commit} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we have talked about this before. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, explicit is better than implicit, and I don't often use abbreviations myself. See above for the reasons and alternatives. |
||
* Python ver: {python_version} | ||
* Qt ver: {qt_version} | ||
* {qt_api_name} ver: {qt_api_version} | ||
* OS: {os_name} {os_version} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please revert this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above. |
||
|
||
### Dependencies | ||
|
||
|
@@ -2496,13 +2515,13 @@ def report_issue(self, body=None, title=None): | |
|
||
@Slot() | ||
def trouble_guide(self): | ||
"""Open Spyder troubleshooting guide in a web browser""" | ||
"""Open Spyder troubleshooting guide in a web browser.""" | ||
url = QUrl(__trouble_url__) | ||
QDesktopServices.openUrl(url) | ||
|
||
@Slot() | ||
def google_group(self): | ||
"""Open Spyder troubleshooting guide in a web browser""" | ||
"""Open Spyder troubleshooting guide in a web browser.""" | ||
url = QUrl(__forum_url__) | ||
QDesktopServices.openUrl(url) | ||
|
||
|
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.
OS
is a term some people could not understand. So please revert this toOperating System:
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.
The problem (and I think I mentioned this before as well) is that we're out of characters to play around with, as aside from a small buffer I built in for one or two deps (and IIRC Linux/Mac has one more line for
psutil
than the Windows I tested it on, and there's an issue to add Matplotlib as a dep) , the report starts getting cut off or outright producing errors when trying to send it to github, and I already cut and cut and cut the issue checklist down to under 1/3 of its original length as well as the disclaimer, headers etc . The real issue is that the dependency list takes up ~3-4x the characters it should due to all the fancy symbols used needed percent encoding; even a small change here (eliminating one or two deps that rarely matter; using one or two fewer unnecessary symbols in the plain text version, etc. will cut dozens or even hundreds of characters from the final URL.Please advise what you'd like me to do—fix the plain text display of the deps? Convert this to use the new error dialog? Just risk it and add back the chars? If I do the latter, adding even one more dep (as is discussed elsewhere) will likely break the whole URL based error reporting mechanism, or at least come dangerously close.
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.
Let me know what you'd like me to do about this, @ccordoba12 . Thanks!
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.
Sorry for the delay in coming back to you. Please remove this entry in the new list:
That's kind of obvious and it should be enough to add the characters that we need.
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.
Sorry for the confusion; I meant to make the above initial comment on the mainwindow review item instead. That entry only exists (in that lengthy form at least) in the issue report template; the relevant text for the Help menu Report Issue item is in
mainwindow
. However,while much shorter, does exist and, once percent encoded, is indeed around the same length as a dependency line, much longer than "versions". Unfortunately it isn't as obvious as one would think as many reporters don't do it, but at least it is something we can easily see for ourselves. I'll go ahead and eliminate it and spell out versions and OS, and push the changes momentarily; that should at least give us a little breathing room for the time being.