clj-commons / camel-snake-kebab

A Clojure[Script] library for word case conversions
https://clj-commons.org/camel-snake-kebab/
Eclipse Public License 1.0
475 stars 49 forks source link

Use best practices in the documentation #59

Closed dotemacs closed 5 years ago

dotemacs commented 5 years ago

What ?

Can't remember where I saw it first, about Clojure libraries in the READMEs, the demonstration of the library usage is still using the deprecated use instead of require.

This PR tries to remedy that for this nifty little library.

The description of the change

As opposed to using

(use 'camel-snake-kebab.core)

use

(require '[camel-snake-kebab.core :as csk])

as its the recommended practice for using Clojure libraries.

See the Clojure Style Guide: https://github.com/bbatsov/clojure-style-guide#prefer-require-over-use

For camel-snake-kebab.core I've abbreviated it to csk. And for camel-snake-kebab.extra I've used cske.

Notes

I've tried every single function in the document in the REPL. And I also got bit by the bean conversion as per: https://github.com/clj-commons/camel-snake-kebab/issues/50 This is while running the project with the profile 1.9.

I picked the abbreviations of csk & cske. But would be quite happy to change them to something else that you might deem suitable.

Let me know what you think, thanks.

qerub commented 5 years ago

use is used to succinctly communicate the library's namespace and function names in a cruft-free manner. It's not an endorsement of sprinkling use in your production code and I don't believe all stylistic best practices have to be followed in example code. That said, I'm out of touch with the Clojure community and would like to hear some more opinions.

@slipset: Can you give this some thought?

I would find (require '[camel-snake-kebab :refer :all]) an acceptable middleground.

dotemacs commented 5 years ago

I tweeted about it, just to get more opinions (without explicitly mentioning this PR):

https://twitter.com/dotemacs/status/1110477283758927873

dotemacs commented 5 years ago

I've also asked on Slack: https://clojurians.slack.com/archives/C03S1KBA2/p1553595856720700

dotemacs commented 5 years ago

A gentle ping...

qerub commented 5 years ago

@dotemacs: Thanks for your involvement. If you have the time, feel free to tweak the page so that "Add the following to your namespace declaration:" renders as list item 2 instead of 1 on https://clj-commons.org/camel-snake-kebab/ . (The issue was already there before your changes and not caused by your indentation changes.)

dotemacs commented 5 years ago

@dotemacs: Thanks for your involvement. If you have the time, feel free to tweak the page so that "Add the following to your namespace declaration:" renders as list item 2 instead of 1 on https://clj-commons.org/camel-snake-kebab/ . (The issue was already there before your changes and not caused by your indentation changes.)

Hey @qerub

I'd love to do it, but not really sure where the issue is. (I mean, I get what you're telling me to do) But if you look at the code, I've bumped up the section numbers in the change that you've merged, but they still show up in 1., 1. sequence.

Here it is as a gist and it renders them in a consecutive order, as 1., 2. (this is as per the PR, that you've merged):

https://gist.github.com/dotemacs/b0032fa56377013c876ecc077845dd8a

Here it is with the spaces removed, in the hope that they introduce this change of the wrong number ordering:

https://gist.github.com/dotemacs/484a9153083dc4c8b7b50c3e8e63f3fd

If you have any suggestions as to how to really fix this, please let me know. Maybe I should just remove the numbering and keep the rest as is?

Thanks

qerub commented 5 years ago

Thanks for looking into it. Removing the numbering instead of hunting down this bug seemed like the most sensible option so I just did so with https://github.com/clj-commons/camel-snake-kebab/commit/9c816d2aeec3f28e5cbbca3d76ed7bd8cf77d659.

dotemacs commented 5 years ago

Great, thanks for that :)

On 8 Apr 2019, at 8:05 pm, Christoffer Sawicki notifications@github.com wrote:

Thanks for looking into it. Removing the numbering instead of hunting down this bug seemed like the most sensible option so I just did so with 9c816d2.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.