-
-
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: Make tour a plugin in the new API #15488
Conversation
Hello @juanis2112! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2021-07-04 23:39:10 UTC |
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.
Thanks @juanis2112 for your work on this! It looks pretty good so far! I left a bunch of suggestions to remove unnecessary code and some questions for you.
Also, please check if you need to reapply the changes added in PRs #15052, #15067, #15036 and #14420, because those were done after the initial migration PR made by Gonzalo.
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
436d73d
to
ddd1093
Compare
ddd1093
to
85574d4
Compare
b426dca
to
26bf96d
Compare
26bf96d
to
7b7ac56
Compare
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.
Great work @juanis2112 so far! Last comments on my side, then this should be finally ready.
spyder/plugins/tours/plugin.py
Outdated
return _("Provide interative tours.") | ||
|
||
def get_icon(self): | ||
return self.create_icon('keyboard') |
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.
@juanis2112, you forgot to address this comment. I suggest adding mdi-map-outline
to the icon_manager dict with the tour
key, and using it here.
Also, please remove the file |
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
8636cb6
to
ec15cf3
Compare
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.
Thanks a lot @juanis2112 for this. You did a great work on this PR!
Description of Changes
This PR is based on #14276
Issue(s) Resolved
Fixes #
Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.
I certify the above statement is true and correct: