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: Add missing signal to convert notebooks from the project explorer #3956

Merged
merged 4 commits into from
Mar 14, 2017

Conversation

andfoy
Copy link
Member

@andfoy andfoy commented Jan 13, 2017

Fixes #3823

@andfoy andfoy added this to the v3.1.1 milestone Jan 13, 2017
@andfoy andfoy self-assigned this Jan 13, 2017
@@ -52,6 +52,8 @@ class Projects(ProjectExplorerWidget, SpyderPluginMixin):
sig_project_loaded = Signal(object)
sig_project_closed = Signal(object)

Copy link
Member

Choose a reason for hiding this comment

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

please remove the space?

@@ -136,6 +138,9 @@ def register_plugin(self):

self.sig_open_file.connect(self.main.open_file)

new_file_f = lambda x: self.editor.new(text=x)
self.sig_new_file.connect(new_file_f)
Copy link
Member

@goanpeca goanpeca Jan 13, 2017

Choose a reason for hiding this comment

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

The idea of lambdas in general is to use them right away and not assign them to a variable, otherwise is better and clearer to use an actual function, so with that spirit in mind...

This is ok

self.sig_new_file.connect(lambda x: self.editor.new(text=x))

@goanpeca
Copy link
Member

goanpeca commented Jan 13, 2017

Thanks for the fix @andfoy, left a couple of comments :-)


Remember to move the PR to the correct Pipe (Ready for review I assume?)

@andfoy andfoy changed the base branch from master to 3.x January 13, 2017 21:44
@goanpeca goanpeca requested a review from ccordoba12 January 13, 2017 21:45
@andfoy
Copy link
Member Author

andfoy commented Jan 13, 2017

There are Conda related errors in both CI engines

@goanpeca
Copy link
Member

@goanpeca
Copy link
Member

goanpeca commented Jan 15, 2017

@andfoy, @ccordoba12 fixed out errors in Appveyor and Travis. Please rebase so we can merge your work in Spyder 3 :-)

@ccordoba12 ccordoba12 closed this Jan 15, 2017
@ccordoba12 ccordoba12 reopened this Jan 15, 2017
@ccordoba12 ccordoba12 closed this Jan 16, 2017
@ccordoba12 ccordoba12 reopened this Jan 16, 2017
@ccordoba12
Copy link
Member

@andfoy, please don't touch this branch until I fix the errors in Appveyor. Those are unrelated to your work.

@ccordoba12 ccordoba12 force-pushed the project_ipynb_conv branch 2 times, most recently from 98376c7 to 9776b01 Compare January 18, 2017 00:54
@ccordoba12 ccordoba12 modified the milestones: v3.1.1, v3.1.2 Jan 18, 2017
@ccordoba12
Copy link
Member

ccordoba12 commented Jan 22, 2017

@goanpeca, we should add a test for this one.

@ccordoba12 ccordoba12 changed the title PR: Successful iPython Notebook conversion from project view PR: Add missing signal to convert IPython notebooks from the project explorer Jan 22, 2017
@ccordoba12 ccordoba12 modified the milestones: v3.1.2, v3.1.3 Jan 22, 2017
@ccordoba12 ccordoba12 removed this from the v3.1.3 milestone Jan 28, 2017
@ccordoba12 ccordoba12 modified the milestones: v3.1.4, v3.1.3 Jan 28, 2017
@ccordoba12 ccordoba12 changed the base branch from 3.x to 3.1.x March 14, 2017 16:05
@ccordoba12
Copy link
Member

I rebased this one and added a test for it. It took me an hour and a half to come up with one!! :-p

But the good thing is that now we're also testing Projects (even if minimally, but it's something ;-)

@ccordoba12 ccordoba12 changed the title PR: Add missing signal to convert IPython notebooks from the project explorer PR: Add missing signal to convert notebooks from the project explorer Mar 14, 2017
@ccordoba12 ccordoba12 merged commit 49cdd71 into spyder-ide:3.1.x Mar 14, 2017
ccordoba12 added a commit that referenced this pull request Mar 14, 2017
ccordoba12 added a commit that referenced this pull request Mar 14, 2017
@andfoy andfoy deleted the project_ipynb_conv branch March 14, 2017 23:17
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.

3 participants