Skip to content
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

Updated Pselect's help to hopefully be clearer and more comprehensive. #3158

Merged
merged 2 commits into from
Sep 3, 2017

Conversation

cianoc
Copy link
Contributor

@cianoc cianoc commented Sep 3, 2017

Just general rewrites, plus the new method wasn't properly documented for Pselect.

@cianoc cianoc changed the title Updated the help to hopefully be clearer and more comprehensive. Updated Pselect's help to hopefully be clearer and more comprehensive. Sep 3, 2017
Copy link
Contributor

@mossheim mossheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm except for two small issues


Values from the source pattern will be passed to strong::func::. Pselect will only return that value if the strong::func:: returns true.

This is the pattern libraries equivalent of link::Classes/Collection#-select#select::.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> library's

@@ -1,19 +1,23 @@
class:: Pselect
summary:: Select values from a pattern
summary:: Filters values from return by a source pattern.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> Filters values returned by a ...

@mossheim mossheim changed the base branch from master to 3.9 September 3, 2017 01:58
Updated Pselect's help to hopefully be clearer and more comprehensive.
Copy link
Member

@LFSaw LFSaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for clarity (and because they are documented in Pattern), could you mark embedInStream and asStream as private?

instancemethods::
PRIVATE:: asStream, embedInStream

@cianoc
Copy link
Contributor Author

cianoc commented Sep 3, 2017

@LFSaw those two functions aren't private, so I don't think that would be the right thing to do.

@telephon
Copy link
Member

telephon commented Sep 3, 2017

It would be god to have a different tag for methods that aren't private but which we want to hide.

hide::

@mossheim mossheim added the comp: help schelp documentation label Sep 3, 2017
@cianoc
Copy link
Contributor Author

cianoc commented Sep 3, 2017

I think hide would be the wrong tag, as really we want this to appear as inherited instance methods, even though they've been overloaded. So I guess what you want is a way of tagging this as a method belonging to Pattern.

By which I mean it's totally fine to use these methods, but it's better that people read about them in the context of Pattern, than here.

@LFSaw LFSaw merged commit 70c9113 into supercollider:3.9 Sep 3, 2017
@LFSaw
Copy link
Member

LFSaw commented Sep 3, 2017

since we agree on the values of these but start to talk about higher-level decisions, I merged.

@LFSaw LFSaw mentioned this pull request Sep 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: help schelp documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants