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

Remove sage-rst2ipynb #23415

Closed
sagetrac-tmonteil mannequin opened this issue Jul 12, 2017 · 17 comments
Closed

Remove sage-rst2ipynb #23415

sagetrac-tmonteil mannequin opened this issue Jul 12, 2017 · 17 comments

Comments

@sagetrac-tmonteil
Copy link
Mannequin

sagetrac-tmonteil mannequin commented Jul 12, 2017

Since rst2ipynb version 0.2.2 supports a command line of the form rst2ipynb input output, there is no longer a need for the sage-rst2ipynb wrapper.

Depends on #23985

CC: @videlec

Component: user interface

Author: Thierry Monteil, Jeroen Demeyer

Branch: c88e146

Reviewer: Sébastien Labbé

Issue created by migration from https://trac.sagemath.org/ticket/23415

@sagetrac-tmonteil sagetrac-tmonteil mannequin added this to the sage-8.1 milestone Jul 12, 2017
@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jul 12, 2017

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jul 12, 2017

Commit: 62c625d

@sagetrac-tmonteil
Copy link
Mannequin Author

sagetrac-tmonteil mannequin commented Jul 12, 2017

comment:2

There should be a way to first print the help and then print the error with some fancy redirections, but unless someone knows how to do it, let us move forward.


New commits:

62c625d#23415 : remove stderr redirection.

@jdemeyer
Copy link

jdemeyer commented Oct 9, 2017

comment:4

Why are you even adding || ( echo -e '\n' ; help ) ? I would go for

exec rst2ipynb --kernel='sagemath' ${1};;

@jdemeyer
Copy link

jdemeyer commented Oct 9, 2017

comment:5

I think that upgrading rst2ipynb to version 0.2.2 (#23985) would remove the need for the sage-rst2ipynb script in the first place.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

Dependencies: #23985

@jdemeyer jdemeyer changed the title Let 'sage -rst2ipynb' output error messages provided by rst2ipynb Remove sage-rst2ipynb Oct 12, 2017
@jdemeyer
Copy link

New commits:

4a31dbfUpgrade rst2ipynb to version 0.2.2 and check pandoc dependency
c88e146Remove sage-rst2ipynb script

@jdemeyer
Copy link

Changed author from Thierry Monteil to Thierry Monteil, Jeroen Demeyer

@jdemeyer
Copy link

Changed commit from 62c625d to c88e146

@seblabbe
Copy link
Contributor

comment:9

I tested sage -rst2ipynb on a file I created and everything is fine. I get All tests passed with make ptestlong. I was going to give positive review, put I have one issue: after installing rst2ipynb, rst2ipynb is not part of my default list of optional packages to test:

slabbe@labbe-laptop sage $ sage -i rst2ipynb
make build/make/Makefile
make[1] : on entre dans le répertoire « /home/slabbe/GitBox/sage »
make[1]: « build/make/Makefile » est à jour.
make[1] : on quitte le répertoire « /home/slabbe/GitBox/sage »
build/bin/sage-logger \
	"cd build/make && ./install 'all-toolchain'" logs/install.log
Nothing to (re)build / all up-to-date.

make build/make/Makefile
make[1] : on entre dans le répertoire « /home/slabbe/GitBox/sage »
make[1]: « build/make/Makefile » est à jour.
make[1] : on quitte le répertoire « /home/slabbe/GitBox/sage »
build/bin/sage-logger \
	"cd build/make && ./install 'rst2ipynb'" logs/install.log
Nothing to (re)build / all up-to-date.
slabbe@labbe-laptop sage $ sage -t --show-skipped src/sage/tests/cmdline.py 
Running doctests with ID 2017-10-19-15-33-52-a53fc644.
Git branch: 23415
Using --optional=ccache,mpir,pandoc_attributes,pandocfilters,python2,sage
Doctesting 1 file.
sage -t --warn-long 53.9 src/sage/tests/cmdline.py
    3 gdb tests not run
    12 internet tests not run
    4 kash tests not run
    6 rst2ipynb tests not run
    [244 tests, 29.19 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 29.2 seconds
    cpu time: 0.5 seconds
    cumulative wall time: 29.2 seconds

why?

@jdemeyer
Copy link

comment:10

Replying to @seblabbe:

after installing rst2ipynb, rst2ipynb is not part of my default list of optional packages to test:

Fixed in #23997

@jdemeyer
Copy link

Reviewer: Sébastien Labbé

@vbraun
Copy link
Member

vbraun commented Oct 21, 2017

@seblabbe
Copy link
Contributor

seblabbe commented Jun 7, 2018

comment:13

follow-up ticket: #25537

@seblabbe
Copy link
Contributor

seblabbe commented Jun 7, 2018

Changed commit from c88e146 to none

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants