#172 sparsersb-1.0.0

[Michele Martone]

I wish to release sparsersb.
It requires librsb-1.2 to build.
librsb is not bundled here (it's a 2MB tarball) although in principle it could.
This has been tested with Octave-4.0.0-rc4.

4fbf4b4ff23c6f269b099cf5d8ca52b1 sparsersb-1.0.0.tar.gz
8c70960613b0f95d828628cf3f4d26bf sparsersb-html.tar.gz

[Carnë Draug]

A couple of things:

1. the html tarball does not have the documentation for any of the functions. It seems to have failed. Look into sparse-html/sparsesb/function. I believe the problem is that INDEX is not up to date.

2. the INDEX file is not up to date

3. you have a PKG_ADD file to add functions with autoload. However, there is no corresponding PKG_DEL which means the functions will remain in the symbol table after unloading the package.

4. you have the PKG_ADD file at the root of the package which means it will be used both in the directory for the m files and oct files. However, this is unique for the oct files, so maybe you could have it as a comment on sparsersb.cc?

5. the autoload, defines a function sparsersb from the sparsersb.oct file. However, an oct file is by default assumed to already have a function with that file name. It seems that the PKG_ADD is not needed at all (point 3 and 4 are no longer necessary then)

6. on the DESCRIPTION file, could you please use "GPLv3+" instead of "GPL version 3 or later". Downstream packagers have complained on our lack of standard license declaration.

7. Man of the files do not have a license header. Could you add a license to all files with source code?

8. I see a configure.base and autogen.sh but no actual configure script.

[Carnë Draug]

Also, would you like me to convert your librsb package to mercurial like I have been doing for most of the other active packages?

[Michele Martone]

Hi Carnë,

thanks for the review.

I respond point to point and attach updated tarballs.

1. The only function which should be documented is sparsersb: now the only file being created is sparsersb-html/sparsersb/function/sparsersb.html . Unfortunately indentation seems wrong there: I suspect something is is not right in the help comment of sparsersb.cc...

2. I adjusted the INDEX file according to http://www.gnu.org/software/octave/doc/interpreter/The-INDEX-File.html#The-INDEX-File

3. Removed PKG_ADD, replaced by "// PKG_ADD" and "// PKG_DEL" in sparsersb.cc

4. As in 3.

5. If this is not OK I'll try check this way as well.

6. Done ("GPLv3+").

7. Added a license similar to that sparsersb.cc to all of the *.m files

8. Now there is one.

With respect to the svn-to-mercurial conversion: let's postpone this until sparsersb is ready first, please.

p.s.:
61769d7bda23af1b8069c3047f349bb5 sparsersb-1.0.0.tar.gz
d6cffe4b39255faf308d2907c7457c59 sparsersb-html.tar.gz

[Carnë Draug]

1 -- all the others files on the html are required. They are the main page of the package online, and the other files are used by scripts to search for functions. Can you please put them back. The only issue was that there was nothing inside the sparsersb-html/sparsersb/function/, all the other files were required too.

3, 4, and 5 -- maybe a missunderstanding. Why do you even have an autoload at all? Does it not work if you drop the PKG_ADD line completely? It should

One other thing which I should have mentioned at the start, can you please add a BuildDepends line to the DESCRIPTION file? About requiring librsb. This is usually the only thing users will read before installing the package.

[Michele Martone]

1. After having followed your instructions this is what gets built; I don't know how to put the other files back --- do you have suggestions ?

3,4, and 5: I confirm sparsersb() works also without the PKG_ADD/PKG_DEL lines in the sparsersb.cc.

Although I don't see "BuildDepends" on
http://www.gnu.org/software/octave/doc/interpreter/The-DESCRIPTION-File.html#The-DESCRIPTION-File
I've added a "BuildDepends: librsb" line. Is this okay or should I have used "BuildRequires" instead ?

[Carnë Draug]

I can't upload the package without the documentation. Does anything gets printed when you generate the html documentation? Any warnings or errors? Anything at all?

You should have used BuildRequires. Apologies for the mistake, I wrote that out of the top of my head, the line is indeed BuildRequires.

[Michele Martone]

I was doing it wrong, forgetting the last argument to
generate_package_html("sparsersb", "sparsersb-html", "octave-forge");
Now it should be better.
Ok, using BuildRequires.
Uploaded updated tarballs.

0144967be900b99d654af767db775772 sparsersb-1.0.0.tar.gz
e232ca658a455ae22f16dc37f6c7931f sparsersb-html.tar.gz

[Carnë Draug]

I have made the release after making some small aesthetic changes to the html.

I looked into your help text to find out why it looked funny and found the issue is that you don't end previous deftypefn blocks before starting a new deftypefnx. This worked fine in TexInfo 4 but will cause issues with TexInfo 5 (and many users now are using TexInfo 5). A deftypefnx macro, must only be used after a deftypefn block, if there's other stuff in between, you must end it first.

Anyway, the octave guidelines is to have all the function signatures at the top and not inlined throughtout the help text. See for example, the help for sparse. The logic behind this is that a quick glance at the top of the function will show you everything, you don't need to scroll through the text to find if there's other ways to call the function. Also, print_usage will only print that header. This is analogous the man pages, all the different ways to call a program are listed at the top on the SYNOPSIS section. I think this is what makes more sense for sparsersb().

Of course there are exceptions. See for example the help text for gallery but that's a very special case and I don't think this is your case. In this cases, you must end deftypefn in the middle of the text before starting a new one.

Finally, if your function signature starts to get too complicated, maybe you should separate into separate functions? For example, rsbwrite (a) or rsbsave (a) instead of sparsersb ("save", a)?