这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

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

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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants