Page MenuHomeSolus

Initial commit of python-pypandoc
Needs ReviewPublic

Authored by abdulocracy on Sun, May 17, 2:06 PM.

Details

Reviewers
JoshStrobl
Group Reviewers
Triage Team
Summary

Initial inclusion of python-pandoc. Required for D8898

Test Plan

Includes unit tests.

Diff Detail

Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

abdulocracy created this revision.Sun, May 17, 2:06 PM
abdulocracy requested review of this revision.Sun, May 17, 2:06 PM
JoshStrobl requested changes to this revision.Thu, May 21, 9:17 AM
JoshStrobl added a subscriber: JoshStrobl.

Your stack is reversed.

package.yml
11

Why does this need networking?

17

Why does this need texlive and texlive-fonts-extra as builddeps? Are they for the check stage? Because we're talking about pulling down over a GB of builddeps just for tests, which is kinda overkill.

This revision now requires changes to proceed.Thu, May 21, 9:17 AM
abdulocracy added inline comments.Thu, May 21, 10:39 AM
package.yml
11

For the check stage yet again, there's a test which pulls a text file to work with.

17

It's for the check stage, but exporting md files to pdf with pandoc is a very common use case, and requires those dependencies, so I thought it would be justified to test at build.

abdulocracy requested review of this revision.Thu, May 21, 10:40 AM
JoshStrobl requested changes to this revision.Thu, May 21, 10:45 AM
JoshStrobl added a subscriber: DataDrake.
JoshStrobl added inline comments.
package.yml
17

They would be rundeps then, not builddeps, if you want it installed with python-pypandoc, since it isn't linking against them. I'm also not really sure having fonts-extras is truly a requirement, I mean why else would @DataDrake separate it out?

This revision now requires changes to proceed.Thu, May 21, 10:45 AM
abdulocracy added inline comments.Thu, May 21, 10:58 AM
package.yml
17

If included as rundeps then any app using just html or whatever else functionality of pandoc would pull in bloat. Apostrophe makes use of pandoc's pdf engine as well as others via python-pypandoc, and tells the user to install LaTeX when they try to export as pdf from the app.

If anything, they could be rundeps of Apostrophe, or any other app that uses the pdf functionality of pandoc, via pypandoc or not. Unfortunately we don't have a way of managing in-between deps like optional deps, so it's either include it or leave it to the user to find out.

Also, there's a font, namely lmodern.sty, that the pdf engine of pandoc requires to work, which is in texlive-fonts-extra. I've had to install that entire package just for pandoc pdf to work for a while, so just the base package is insufficient for the functionality.

abdulocracy requested review of this revision.Thu, May 21, 10:58 AM