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

Cleanup nbconvert templates #3764

Merged
merged 2 commits into from
Jul 30, 2013
Merged

Cleanup nbconvert templates #3764

merged 2 commits into from
Jul 30, 2013

Conversation

damianavila
Copy link
Member

The templates have a lot of whitespaces, some missing ones... and there is no a pythonic structure when you read the code (some indentations missing an so on...)
So, I propose a clean up of the templates... I think this PR make them more readable and easy to understand for people who want to learn to write their own templates.
Maybe I am missing some details deleting white lines, so I open this PR to discuss and review the changes.

Cheers.

Damián.

@Carreau
Copy link
Member

Carreau commented Jul 25, 2013

At the beginning the newlines /whitespace where to keep a perfect matching between nbconvert 1 and 2, so I have no opposition in cleaning them, and thanks a lot for that. So I don't have any objection for HTML ans Latex template,
I would just like to check on markdown and rst is the rendered document look natural and do not have too many extra line.

Travis build errored, I just relauched.

Tagging 1.0

@damianavila
Copy link
Member Author

For reference, I have not touched the latex templates because I did not know if the current structure is necessary to render ok...
I can do reorganization of latex templates but I will need some eyes to see in depth ;-)

@damianavila
Copy link
Member Author

OK, I added some fixes in latex templates to be congruent with the others... I did not have to do a lot of modifications because there was more organized than the non-latex templates.

@Carreau
Copy link
Member

Carreau commented Jul 25, 2013

hum... need rebase. But you might want to wait for #3758 to land.

@damianavila
Copy link
Member Author

I will rebase it in a couples of minutes... I am pushing you another PR ;-)

@damianavila
Copy link
Member Author

@Carreau rebase done ;-)

@jdfreder
Copy link
Member

Looks like it needs another rebase 😦

@damianavila
Copy link
Member Author

I will do it in a couple of minutes

@damianavila
Copy link
Member Author

Fixing rebasing conflicts, don't review yet... thanks.

@damianavila
Copy link
Member Author

OK, I don know why travis is failing, probably it has to be run again... I am not familiarize with travis but I can run it again or some of you have to do it?

@minrk
Copy link
Member

minrk commented Jul 29, 2013

re-ran travis, wasn't an issue here. Personal testing, and this seems to be working fine for me. 👍 to merge, but I'll let @Carreau or @jdfreder push the big green button.

jdfreder added a commit that referenced this pull request Jul 30, 2013
@jdfreder jdfreder merged commit fe0e3e5 into ipython:master Jul 30, 2013
@jdfreder
Copy link
Member

Good work @damianavila . I like these kinds of aesthetic changes :)

@damianavila damianavila deleted the fix_templates branch July 30, 2013 05:54
@damianavila
Copy link
Member Author

Thanks!

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
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