Solene Rapenne wrote:
> On Thu, 16 Sep 2021 19:33:07 +0100
> Stuart Henderson <[email protected]>:
> 
> > On 2021/09/16 19:02, Omar Polo wrote:
> > > 
> > > Solene Rapenne <[email protected]> writes:
> > >   
> > > > On Thu, 16 Sep 2021 09:04:37 +0200
> > > > Omar Polo <[email protected]>:
> > > >  
> > > >> Solene Rapenne <[email protected]> writes:
> > > >>   
> > > >> > Import a simple python dependency for editors/apostrophe
> > > >> >
> > > >> > I generated it with portgen, checked version, DESCR, PLIST, 
> > > >> > licensing.
> > > >> > I didn't add pandoc as a dependency because it's not required to
> > > >> > have it at build stage, and I'm not sure it would be fine to add
> > > >> > pandoc as a run dep either.    
> > > >> 
> > > >> without text/pandoc as TEST_DEPENDS the tests fails; well, they fails
> > > >> nevertheless because it tries to fetch stuff from github.  No idea how
> > > >> to fix it, sorry.
> > > >> 
> > > >> Anyway, I'd argue that it needs text/pandoc as {RUN,TEST}_DEPENDS and
> > > >> not RDEP on it from apostrophe since it searches for the binary and
> > > >> throw an error if not found.  Attaching another tarball with pandoc
> > > >> added as deps.
> > > >> 
> > > >> Cheers,
> > > >>   
> > > >
> > > > I added TEST_DEPENDS , it can make 23 checks before failing
> > > > when trying to download its own sources to use the README.md file...
> > > > but I suppose 23 checks working is better than nothing.  
> > 
> > No need to list RUN_DEPENDS in TEST_DEPENDS
> > 
> > > I was distracted by the error output and the /writes_to_HOME lines.  It
> > > seems that all but that test are succeeding, which is good I guess.  
> > 
> > PORTHOME=${WRKDIR} probably helps with the /writes_to_HOME
> > 
> > > > It's now depending on pandoc for RUN_DEPENDS  
> > > 
> > > I've seen these two nits only now, but I'm not familiar with the python
> > > infrastructure in ports so they may be wrong/opinable:
> > > 
> > >  - comment starts with uppercase; also maybe specify that's a python
> > >    wrapper?  
> > 
> > Python is a proper noun so uppercase is preferable there
> > 
> > >  - use ``py-wheel${MODPY_FLAVOR}'' instead of ``py-wheel,python3''  
> > 
> > correct
> > 
> > > If by any chance I'm not wrong, here's an updated tarball.
> > > 
> > > Cheers!
> > >   
> > 
> 
> new tarballs attached
> 
> - py-pypandoc comment has now uppercased Python, added PORTHOME
> - removed a trailing whitespace in apostrophe
> 
> Apostrophe complains about missing texlive for exporting PDF but it
> does so with a nice GUI explaining what is missing. I prefer not add
> texlive as a run dependency, ok?

For apostrophe:

You're getting it from github, which is a mirror. The primary source 
is on gitlab. Would it make sense to set the gitlab URL as homepage? 
This is where issues etc. are hosted.

Nitpick: You could reduce the indentation by one level for all 
entries.

For py-pypandoc

attached again with the following changes:
- Added test dependency devel/py-pip.
- Patched the failing test case, so it runs with a local file:// url.
  The README.md is already present, and the file:// url should run 
  the same code path within py-pypandoc.

Best regards,
Stefan

Attachment: py-pypandoc.tgz
Description: application/tar-gz

Reply via email to