clojure / clojure-site

clojure.org site
https://clojure.org
Eclipse Public License 1.0
249 stars 270 forks source link

Update protocols.adoc #654

Open ieugen opened 1 year ago

ieugen commented 1 year ago

Made sure this is mentioned as first argument. It's not obvious from the docs.

Signed CA on 2022-11-16 .

seancorfield commented 1 year ago

Because you've added an extra argument, the example calls are no longer correct.

Perhaps renaming the current first argument to this would be a safer change? (although that would make the argument lists be this b and this b c which is less intuitive in my opinion)

ieugen commented 1 year ago

I can prepend this and drop the last argument.

ieugen commented 1 year ago

@seancorfield : If it's still not good, please feel free to update it so it is fine.

seancorfield commented 1 year ago

The protocol P / bar-me code is still incorrect because you've added an argument there.

I'll leave it up to @puredanger et al to decide what clarifications actually work / are needed here.

puredanger commented 1 year ago

There are a couple of existing issues, #215 and #216, that I think also have some excellent points and examples. I think the most important thing to change about the examples here is to make the examples actually meaningful, and not use foo/bar/baz at all. Or maybe it's syntax + examples or something.

My one hesitation with making this more prevalent (even though this is a common usage), is that it implies extra meaning for developers coming from Java (where that is a special thing), and also the anaphoric macro proxy where this actually is a "special" bound symbol. But maybe it's just a matter of saying that explicitly in the doc.

ieugen commented 1 year ago

I think keeping this (for a lack of better name) + explicit mentioning behavior in the docs is good enough.
I could try to merge the docs. @puredanger: please let me know if you want to take this on or not.