tech-pkg archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: request review: blogc



Todd MARTIN <kj4ntv%gmail.com@localhost> writes:

>> It looks very good; pkglint shows only one issue, the lack of COMMIT_MSG
>> (that could be used to add it to pkgsrc with "cvs commit -F COMMIT_MSG".
>
> Added this file in. Was a little confused initially because I don’t
> have really experience working with CVS and using git with pkgsrc-wip,
> but I was able to find example in other packages in pkgsrc-wip to get
> an idea of what I need to do here.

We don't really document this, but commit messages for new packages also
include (at least part of) DESCR; I just stuck that in.

> I adjusted the description, hopefully that helps a little more. So
> give some context, blogc is pretty much a static site generator, I
> used the “blog compiler” terminology as that is what upstream uses. I
> also don’t have a problem switching the language to just say. A static
> site generator.

Thanks; I also tweaked COMMENT to give people the hint that this is a
single-page tool, not a full site generator like hugo.  (Yes, I know
people can use make/etc. to build multiple pages.  It's still
different.)

I realized that the tests use bash.  IMHO that's an upstream bug unless
there's a really good reason POSIX shell couldn't be made to work.  It's
also an upstream bug not to be documented.  Plus it uses diff and tee.
I added a USE_TOOLS line for these.

I've imported it to www/blogc and removed the wip directory.

If you're feeling double-plus diligent, you might file upstream bugs:

  - README says "blog compiler" but the program is a blog entry
    compiler.  The point is that the word blog refers to a whole site or
    sub-site with multiple pages and navigation among them, not a single
    page.  And, blog is perhaps overly specific, if this can compile web
    pages that aren't really blog pages.
  - README doesn't document that c99 is required.
  - README doesn't document how to run tests.
  - README doesn't document tests dependencies.
  - Tests need bash (should use POSIX shell as /bin/sh instead ).
  - Tests are apparently run with "set -x" and are thus overly verbose.

None of these are super serious, but they'll help the next packager on
$OTHER_OS.


Home | Main Index | Thread Index | Old Index