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

Documenting function parameter types and returns #1071

Merged
merged 1 commit into from
Jun 16, 2015
Merged

Conversation

captncraig
Copy link
Contributor

This also includes formally renaming Number to NumberSet to cause less confusion.

Review on Reviewable

@kylebrandt
Copy link
Member

This addresses #1026


Change the NaN value during binary operations (when joining two queries) of unknown groups to the scalar. This is useful to prevent unknown group and other errors from bubbling up.

## sort(number, asc_desc)
## sort(numberSet, asc_desc string) numberSet
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a format when strings have a predefined set of values. So for sort the asc desc, lsstat is has a set as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure how to define it. The description makes it pretty clear what to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

something like regex syntax: /"(asc|desc)"/

or Powershell validation sets: [ValidateSet("Steve","Mary")]

Copy link
Contributor

Choose a reason for hiding this comment

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

And I disagree that the description is clear. You should be able to call a function knowing only it's signature, and asc_desc is not a defined type. If you want to do that you should do what .NET and Go do and use enums or consts instead of strings

Copy link
Member

Choose a reason for hiding this comment

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

I like the Piped version (Minus the escapes and quotes, so

(asc|desc) string

On Mon, Jun 15, 2015 at 3:40 PM, Greg Bray notifications@github.com wrote:

In docs/expressions.md
#1071 (comment):

Change the NaN value during binary operations (when joining two queries) of unknown groups to the scalar. This is useful to prevent unknown group and other errors from bubbling up.

-## sort(number, asc_desc)
+## sort(numberSet, asc_desc string) numberSet

something like regex syntax: "(asc|desc)"\

or Powershell validation sets: [ValidateSet("Steve","Mary")]


Reply to this email directly or view it on GitHub
https://github.com/bosun-monitor/bosun/pull/1071/files#r32475366.

@kylebrandt
Copy link
Member

@captncraig Looking good - much appreciated. Ship at will

@gbrayut
Copy link
Contributor

gbrayut commented Jun 16, 2015

Agreed... looks much better! Great work!

captncraig added a commit that referenced this pull request Jun 16, 2015
Documenting function parameter types and returns
@captncraig captncraig merged commit 1cc2fd6 into master Jun 16, 2015
@captncraig captncraig deleted the funcSigs branch June 16, 2015 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants