New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check public functions for public examples #4998
Conversation
8f0be2f
to
65cfc26
Compare
|
1514e4f
to
42a14fe
Compare
87017a5
to
cb0843e
Compare
cb0843e
to
054c3da
Compare
054c3da
to
9ef2f5c
Compare
Rebased to
|
Cool! This is getting close to https://issues.dlang.org/show_bug.cgi?id=16614 as well. |
std/digest/digest.d
Outdated
@@ -840,7 +840,7 @@ ref T[N] asArray(size_t N, T)(ref T[] source, string errorMsg = "") | |||
return *cast(T[N]*) source.ptr; | |||
} | |||
|
|||
/** | |||
/* | |||
* This helper is used internally in the WrapperDigest template, but it might be | |||
* useful for other purposes as well. It returns the length (in bytes) of the hash value | |||
* produced by T. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That change might be controversial, but there weren't any tests for digestLength
and the documentation starts with "this helper is used internally", so this change wouldn't expose it on dlang.org.
What's your opinion on this @jpf91? Should we better reword the documentation and add tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we better reword the documentation and add tests?
Yes, I think that's a better solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that's a better solution.
Okay, thanks!
I removed the bit about "used internally only" and added a simple unittest
.
Mind the style failures |
-> #5215 (though there's one in here as well) |
To improve Phobos's documentation, here's another addition to the
style
Makefile target.Here's the idea that every public function should have at least one public example as stated in the high-level vision (btw for all public examples the
publictests
Makefile target we will enforce that it's runnable on dlang.org)Script is at the tools repo.
Note: this just adds the tool and comes with quite a long blacklist. While I started to fix some modules (e.g. #4973 and trivial ones in this PR), it's quite a lot of effort. However, once this has been enabled, everyone should be able to help to reduce the blacklist :)