atlas-engineer / nfiles

User configuration and data file management
BSD 3-Clause "New" or "Revised" License
17 stars 5 forks source link

Update NASDF to 1.3. #21

Closed aartaka closed 1 year ago

aartaka commented 1 year ago

There's a lot of errors raised by undocumented exports test. @Ambrevar, want me to fix these?

Ambrevar commented 1 year ago

Something is off with the compilation test. For instance read-handler has a documentation string. My guess is that slot documentation is not found.

If you know which class the slot belongs to, then it's easy to check for documentation:

(getf (mopu:slot-properties 'CLASS-NAME 'SLOT-NAME) :documentation)

However, how can we find the associated class(es)?

aartaka commented 1 year ago

Something is off with the compilation test. For instance read-handler has a documentation string. My guess is that slot documentation is not found.

Yes, slot documentation has no standard way for documentation fetching (see standard page for documentation). We can go through all the classes in the package and look at their slots documentation via MOP. However,

So using MOP is not an option.

That's why there are two exceptional cases to the documentation test, both with respective ways to handle them:

Ambrevar commented 1 year ago

Maybe better: If symbol is not fbound, bound, a class, etc. (see other compilation test), then do not check the doc.

aartaka commented 1 year ago

Maybe better: If symbol is not fbound, bound, a class, etc. (see other compilation test), then do not check the doc.

Most of the exported slots also have accessors, so that doesn't work as a heuristic. And then, if the symbol is not bound as per ubound-exports, then it's a whole different class of errors :D

Ambrevar commented 1 year ago

Then I don't find the documentation check very satisfactory, since maintaining this list of symbols to ignore could itself lead to mistakes.

aartaka commented 1 year ago

Then I don't find the documentation check very satisfactory, since maintaining this list of symbols to ignore could itself lead to mistakes.

Yeah, that's not cool, but this test is still a progress for all the cases but slots.

aartaka commented 1 year ago

Okay, I've patched the tests to not check accessor functions (as these are often auto-generated). Now the list of undocumented symbols is much saner:

; Errors:
((#<PACKAGE "NFILES">
  (EXTERNAL-MODIFICATION PROCESS-ERROR INVALID-CHECKSUM *DEFAULT-PROFILE*
   EXISTS-P FETCH-ERROR))
 (#<PACKAGE "NFILES/GPG">
  (*GPG-DEFAULT-RECIPIENT* *GPG-PROGRAM* GPG-UID GPG-KEY)))
Found undocumented exported symbols in 2 packages.
aartaka commented 1 year ago

Is it NASDF 0.1.4 then?

Ambrevar commented 1 year ago

I like the result after skipping accessors, but we should fix the code.

Care to fill up the missing docstrings? Otherwise I can do it later.

aartaka commented 1 year ago

I'll get back to this tomorrow, polishing things up, using built-in MOP thingie, and fixing up the docs!

aartaka commented 1 year ago

All done, merging? I'll squash and rebase, obviously. The only undiscussed thing is whether we should retain :undocumented-symbols-to-ignore, but I believe there are enough exceptional situations (as there are more than one namespace in CL) to keep it, just in case.

aartaka commented 1 year ago

Merged!