Skip to content

Update protocols.adoc #654

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update protocols.adoc #654

wants to merge 2 commits into from

Conversation

ieugen
Copy link

@ieugen ieugen commented Jul 15, 2023

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

  • Have you read the guidelines for contributing?
  • Have you signed the Clojure Contributor Agreement?
  • Have you verified your asciidoc markup is correct?

Signed CA on 2022-11-16 .

Made sure `this` is mentioned as first argument.
It's not obvious from the docs.
@seancorfield
Copy link
Member

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
Copy link
Author

ieugen commented Jul 16, 2023

I can prepend this and drop the last argument.

@ieugen
Copy link
Author

ieugen commented Jul 16, 2023

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

@seancorfield
Copy link
Member

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
Copy link
Member

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
Copy link
Author

ieugen commented Jul 18, 2023

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants