Clojure

Improve clojure.zip documentation

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: Release 1.8, Release 1.9
  • Fix Version/s: None
  • Component/s: None
  • Labels:
  • Environment:
    All
  • Patch:
    Code
  • Approval:
    Triaged

Description

The clojure.zip module has extremely terse docstrings that are helpful as reference material but completely unhelpful to someone approaching the module cold. Expanded docstrings would make this piece of code much more visible to users who might find it helpful.

Activity

Hide
Brighid M added a comment -

A patch that improves clojure.zip's docstrings.

Show
Brighid M added a comment - A patch that improves clojure.zip's docstrings.
Hide
Alex Miller added a comment -

I would prefer if we could minimize the size of the changes by removing diffs that just add periods, change line breaks, add whitespace, or make other non-essential changes. That will help focus on the things that matter like changes in the ns docstring, zipper, etc. Additionally this ticket talks about doc strings but also makes changes in exception messages - I'd prefer code changes to be in a separate ticket. Also, please don't remove the comment sections in the files.

If you would like to convert the test comment section into actual tests, that would be a great separate ticket (I feel like I might have even written a patch to do that at one point but I don't see a ticket for it!).

Keeping this patch entirely focused on essential docstring changes is the best way to ensure its timely inclusion.

Show
Alex Miller added a comment - I would prefer if we could minimize the size of the changes by removing diffs that just add periods, change line breaks, add whitespace, or make other non-essential changes. That will help focus on the things that matter like changes in the ns docstring, zipper, etc. Additionally this ticket talks about doc strings but also makes changes in exception messages - I'd prefer code changes to be in a separate ticket. Also, please don't remove the comment sections in the files. If you would like to convert the test comment section into actual tests, that would be a great separate ticket (I feel like I might have even written a patch to do that at one point but I don't see a ticket for it!). Keeping this patch entirely focused on essential docstring changes is the best way to ensure its timely inclusion.
Hide
Brighid M added a comment -

Alex — to clarify, I'm hearing "this ticket should include two patches: one with the major prose additions and one with small proofreading-ish changes" and "this ticket's patches should not include the changes to exception messages nor the comment-move"?

Supplemental question: is there a style guide hanging around for the code & documentation style of the Clojure core? I poked around for such a guide on clojure.org and in JIRA: I didn't find one, but maybe I overlooked it. I was looking for a guideline about the comment section. It would be good to have a hint about "please don't touch these," because AFAICT they don't communicate that by themselves.

Show
Brighid M added a comment - Alex — to clarify, I'm hearing "this ticket should include two patches: one with the major prose additions and one with small proofreading-ish changes" and "this ticket's patches should not include the changes to exception messages nor the comment-move"? Supplemental question: is there a style guide hanging around for the code & documentation style of the Clojure core? I poked around for such a guide on clojure.org and in JIRA: I didn't find one, but maybe I overlooked it. I was looking for a guideline about the comment section. It would be good to have a hint about "please don't touch these," because AFAICT they don't communicate that by themselves.
Hide
Alex Miller added a comment -

Actually, I'd prefer to have just one patch with the major prose additions. I don't think the minor changes are worth doing and will be a distraction from the more important prose.

Unfortunately, there is no style guide for Clojure core, and in fact it's been written by many people over many years so there generally isn't a consistent style throughout the code. Generally Rich prefers that debug code or comments that trace to him are left intact (git blame can help there). More generally, the simpler a patch is to review, the easier it is for it to stay good and be easily reviewed.

Thanks!

Show
Alex Miller added a comment - Actually, I'd prefer to have just one patch with the major prose additions. I don't think the minor changes are worth doing and will be a distraction from the more important prose. Unfortunately, there is no style guide for Clojure core, and in fact it's been written by many people over many years so there generally isn't a consistent style throughout the code. Generally Rich prefers that debug code or comments that trace to him are left intact (git blame can help there). More generally, the simpler a patch is to review, the easier it is for it to stay good and be easily reviewed. Thanks!
Hide
Brighid M added a comment -

> Actually, I'd prefer to have just one patch with the major prose additions.
Okay, I'll produce that patch and attach it to this ticket.

> I don't think the minor changes are worth doing and will be a distraction from the more important prose. Unfortunately, there is no style guide for Clojure core, and in fact it's been written by many people over many years so there generally isn't a consistent style throughout the code.
I think you just made an argument for why it is worth doing minor changes: consistency generally makes things more accessible. However, now is clearly not the time to litigate that, so I'm gonna come back with just the ns/docstring version of this patch.

Show
Brighid M added a comment - > Actually, I'd prefer to have just one patch with the major prose additions. Okay, I'll produce that patch and attach it to this ticket. > I don't think the minor changes are worth doing and will be a distraction from the more important prose. Unfortunately, there is no style guide for Clojure core, and in fact it's been written by many people over many years so there generally isn't a consistent style throughout the code. I think you just made an argument for why it is worth doing minor changes: consistency generally makes things more accessible. However, now is clearly not the time to litigate that, so I'm gonna come back with just the ns/docstring version of this patch.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated: