Skip to content
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: Feature/add state handling #43

Merged
merged 5 commits into from
Mar 22, 2016
Merged

Conversation

goanpeca
Copy link
Member

This PR completes the expected behavior of icons, which include the possibility of a State (On, Off)

Added new example to illustrate usage.

@goanpeca
Copy link
Member Author

@SylvainCorlay we kind of need this urgently, any chance you get to review today?

@SylvainCorlay
Copy link
Member

Hi Gonzalo,

I can look at it tonight but if you really need it merged now to iterate,
you should gi ahead and hit the green button.

Cheers,

S.
On Mar 10, 2016 9:49 AM, "Gonzalo Peña-Castellanos" <
notifications@github.com> wrote:

@SylvainCorlay https://github.com/SylvainCorlay we kind of need this
urgently, any chance you get to review today?


Reply to this email directly or view it on GitHub
#43 (comment).

@goanpeca
Copy link
Member Author

@SylvainCorlay what happen with I can take a look at it later tonight 😆?, its ok, been there as well, right @Nodd?

Anyway, we are using our own icons (svg) directly, so this change is no longer critical... still I would appreciate if you took some time to review it as this covers an important use case.

Thanks 😸

@SylvainCorlay
Copy link
Member

Hi I am travelling this week so I won't be able to look at it right away. If you are confident, you can go ahead and merge. Regarding the svg icons, is it something you think should/could be integrated in the spyder font?

@goanpeca
Copy link
Member Author

Regarding the svg icons, is it something you think should/could be integrated in the spyder font?

Nope this is very specific to the application we are developing and not related to Spyder.

import json
import os

# Third party importd
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in "importd"

@ccordoba12
Copy link
Member

I addressed all @Nodd's comments, so I'm merging :-)

ccordoba12 added a commit that referenced this pull request Mar 22, 2016
@ccordoba12 ccordoba12 merged commit 2186220 into master Mar 22, 2016
@ccordoba12 ccordoba12 deleted the feature/add-state-handling branch March 22, 2016 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants