Faré wrote: > Dear Robert, > > here are some code review comments on ASDF 3.0.2.1.
Thank you very much for these comments. They are hugely helpful, and I will act on them. > > * Putting :asdf/find-system after :asdf in the recycle looks like it's > defeating the purpose; > you might as well remove it. Will do. I didn't get around to documenting RECYCLE per our conversation on the ticket. I have pushed a revision (3.0.2.2) with documentation, perhaps you could review? In particular, I don't adequately understand the :MIX option, so have left it without documentation. Would it be reasonable to have DEFINE-PACKAGE have keyword arguments as well as the &rest parameter? The idea would be to make the information an emacs (or other) environment displays to the programmer be more useful. Another related question: in ENSURE-PACKAGE, there's a case where we ignore names that the programmer has asked us to UNINTERN (when they are :INHERITED). Question: should we be signaling this with a WARNING, in case the user's expressed intent is being violated? Or is this something that must be violated sometimes in order to effectively upgrade (in which case we should leave this here). > * Maybe refactor duplicate-names so it doesn't inherit from > system-definition-error ? > Or have a function of the same name be called that is defined later > to throw the error? That seems possible, or perhaps we should just have an ASDF/CONDITIONS package and kick *all* of the conditions up the dependency order? If you want one, just import it or use this package. That seems possibly to cause upgrade issues, though, so I have left this undone. > * Since we don't work on antique cmucl, maybe we could check whether > the :report print-object hack is still needed? I haven't been testing on either cmucl or abcl, it seems. I will restore those, and maybe we could do this. Raymond seems like the authority on this. > * I kind of wanted to prevent the multiplication of .asd files and the > eating away of the namespace by using test-asdf/foo systems for new > systems; but it's your baby, so your call. Also, an error might not be > compatible with putting it in test-asdf.asd. On the other hand, that's > a case where define-test-system and/or eval can be used so you don't > need a .asd file at all. Thank you; I will think about making these changes. > > * the nodes in the grammar could be renamed to be less confusing and > not require a comment. Agreed. Also, I'm not sure about the syntax that's used here. There are some uses of @var markup, but it's not clear to me what that signifies. I suspect we haven't used it consistently as the texinfo has accumulated over time. The manual really needs an end-to-end overhaul, but I don't see myself as having the time to do this in the near future. best, r > > —♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org > The politicians have a most touching faith in technology — that it can make > up for any dumb thing the politicians decide to do. — John McCarthy
