search

anaconda / nbpresent

next generation slides for Jupyter Notebooks
BSD 3-Clause "New" or "Revised" License
162 stars 23 forks source link

Package review comments #33

Closed ijstokes closed 8 years ago

ijstokes commented 8 years ago

I just did a quick review of the package (not code level, but packaging), and noticed a few things:

bollwyvl commented 8 years ago

Thanks for the feedback!

it is used here: https://github.com/Anaconda-Server/nbpresent/blob/master/conda.recipe/run_test.sh#L3

To get the stars of phantomjs, casperjs, npm, conda and the notebook aligned, i basically need to run the notebook tests from inside npm. In the test environment, I don't need the whole build chain, just a few packages. this extra package.json handles both of these.

  • Do we really want to have README.rst and README.md and README.html all checked in?

The html is an oversight, but the other two: maybe. They are both generated from notebooks/README.ipynb, but the rst is used exclusively at the time of a pypi release. since we've basically got that automated anyway, we can likely safely ignore it...

Unclear to me why this line is required: https://github.com/Anaconda-Server/nbpresent/blob/master/conda.recipe/build.sh#L6

I believe this was to appease sphinx... i'll have to revisit.

If it is possible to fix this PIP entry in environment.yml that would be great: https://github.com/Anaconda-Server/nbpresent/blob/master/conda.recipe/build.sh#L6

Not sure if this is the link you meant... how is the entry point breaking?

bollwyvl commented 8 years ago

re: readme.ipynb: Looks like the ipynb->markdown isn't working properly either.

damianavila commented 8 years ago

I think the discussion got stale by now... closing.