probcomp / metaprob

An embedded language for probabilistic programming and meta-programming.
GNU General Public License v3.0
168 stars 17 forks source link

What's going on with metaprob.examples.all-test? #67

Closed zane closed 5 years ago

zane commented 6 years ago

Right now all it contains is:

(defn foo [] 'hello)

I'm guessing that the idea was that it would at some point replace metaprob.examples.main?

jar398 commented 6 years ago

Well, as you might expect, there is some history to this, and the explanation is to be found in the history.

I made a version of the all module, which essentially does nothing other than a bunch of requires, and one day it failed because there was a mistake in one of these requires. So, I added a test to force lein test to at least try to load the module, so that any errors loading would be flagged.

Since there is no relationship to main I don't think there should be any such connection.

As far as I'm concerned the test should stand pretty much as it is since it does exactly what was intended and I can't think of anything else it should do. If there is to be a change, it would be the addition of a comment explaining this.

all is a user interface feature and should not be used for any other purpose than setting up a REPL for use by people who don't know about, or don't want to be bothered with, using require or use. Nobody should ever require (use, etc.) all. Because it contains no code, there can be no tests of it other than whether it loads.

jar398 commented 6 years ago

Also, the reason the defn foo is there is that Clojure didn't accept the file if there was nothing there (which is what I wanted to do).

zane commented 6 years ago

Instead of adding comment would it be acceptable to replace (def foo …) with a test that runs (require 'metaprob.examples.all)? It might make the intent clearer.

jar398 commented 6 years ago

That's a fine idea.