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

Implement #227 #228

Merged
merged 4 commits into from
Mar 1, 2017
Merged

Implement #227 #228

merged 4 commits into from
Mar 1, 2017

Conversation

mottosso
Copy link
Member

@mottosso mottosso commented Mar 1, 2017

Alert the window on controller.finished signal

@mottosso
Copy link
Member Author

mottosso commented Mar 1, 2017

Thanks @BigRoy, should we try and get the balloon popup in there as well? Or better save that for a next PR?

@BigRoy
Copy link
Member

BigRoy commented Mar 1, 2017

This is working great as a minor feature actually, because of its simplicity I assume this could be part of a patch version and separate out the notification. I'm expecting that to have some discussion along the way, regarding how long it stays, design, what's messaged in it and how. That would mean this minor feature will only be waiting longer than needed (and could stay off from merging until we've gone fully through the notification feature).

How about renaming #227 to alert user on completion and make a new issue regarding show notification pop-up upon completion to really have them separated?

@mottosso
Copy link
Member Author

mottosso commented Mar 1, 2017

How about renaming #227 to alert user on completion and make a new issue regarding show notification pop-up upon completion to really have them separated?

Agreed, this is such a simple and small modification that it stands well on its own.

@mottosso
Copy link
Member Author

mottosso commented Mar 1, 2017

Version up please?

@BigRoy
Copy link
Member

BigRoy commented Mar 1, 2017

I happened to have merged with this repo just now, and added something to .gitignore. So note the additional commits above.

@mottosso
Copy link
Member Author

mottosso commented Mar 1, 2017

No problem.

@mottosso mottosso merged commit 60b7bad into pyblish:master Mar 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants