tolitius / mount

managing Clojure and ClojureScript app state since (reset)
Eclipse Public License 1.0
1.23k stars 89 forks source link

using mount.core instead of just mount would be more idiomatic #11

Closed yogthos closed 8 years ago

yogthos commented 8 years ago

it's a small nitpick, but it's a bit more idiomatic to create a core namespace

tolitius commented 8 years ago

I hope it is "small" :)

For some reason core for the sake of core never sat well with me. I agree that most of the iconic projects use it, but.. what I noticed, once additional features are added to the project, core goes away, or becomes really small (i.e. 1, 2), and real names pop up, or projects :) just start without core all together, which I think is quite clean.

yogthos commented 8 years ago

The official naming convention as well as the style guide suggest that namespaces should use at least two parts in the name. I figure it's good for a library to follow common conventions. :)

tolitius commented 8 years ago

A lib name is a symbol that will typically contain two or more parts separated by periods

and

it's good for a library to follow common conventions

sound convincing... but :sad: :) I don't understand why can't a library "be named by its name"

yogthos commented 8 years ago

I think the original intent was to make namespace collisions less likely, and then people started using lib.core as a convention when there wasn't an obvious second symbol for the name.

One advantage of this convention is that it makes it easy to find the entry point when looking through the code. In the om/reagent examples the core gives a starting point to trace from for example.

tolitius commented 8 years ago

makes sense, but still feels off. "wasn't an obvious second symbol for the name" usually occurs when something is wrong with naming :)

but again, I agree. this is not huge to fight the system. and adding .core most likely would only help rather than hurt, since it is expected.

yogthos commented 8 years ago

yeah pretty much :)

tolitius commented 8 years ago

found an interesting :) related commit:

Note: Since DataScript 0.13 main namespace to include is datascript.core, not datascript. This was done because usage of top-level namespaces is discouraged and even generates a warning in CLJS. Old datascript.core, if you were using it, was renamed to datascript.db

yogthos commented 8 years ago

looks like many projects end up going through this :)

yogthos commented 8 years ago

and I've integrated mount with Luminus, it fits like a glove. Once the new version with the API update is up on Clojars I'd like to push out a new version of Luminus using it.

The main icky part for me was having global defs for things like db connections, and since I'm trying to be beginner friendly Component was out. With Mount I can just replace my connection var with one using defstate and it's beautiful. :)

tolitius commented 8 years ago

this is great, thank you for the feedback! :)

yogthos commented 8 years ago

Any chance of a new release with the change, I'd like to push out a new luminus release with mount integrated and I want to target the new API from the get go.

tolitius commented 8 years ago

sure: 0.1.3

yogthos commented 8 years ago

:+1: