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

Implement support for optionally specifying the 'var' keyword in 'for' loops #6110

Merged
merged 1 commit into from Jul 26, 2018

Conversation

gunnarbeutner
Copy link
Contributor

This adds support for optionally using the var keyword in for loops:

for (var number in [ 4, 8, 15, 16, 23, 42 ]) {
    log(number)
}

and

for (var key => var value in globals) {
    log(key)
    log(typeof(value))
}

Adding the var keyword has no semantic effect other than unifying how we usually do variable declarations.

@gunnarbeutner gunnarbeutner added the area/configuration DSL, parser, compiler, error handling label Feb 22, 2018
Copy link
Contributor

@Crunsher Crunsher left a comment

Choose a reason for hiding this comment

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

lgtm 👯‍♀️

@dnsmichi
Copy link
Contributor

From a first look, this makes it complicated to follow and understand e.g. apply for inside the monitoring basics chapter. Let's talk about this next week in the office :)

@dnsmichi dnsmichi added the needs feedback We'll only proceed once we hear from you again label Feb 28, 2018
@dnsmichi
Copy link
Contributor

It can be added, nevertheless we shouldn't change our entire documentation for it. If that's the requirement, I'd drop this feature as this makes it complicated for our users (who are not programmers).

@dnsmichi dnsmichi added TBD To be defined - We aren't certain about this yet and removed needs feedback We'll only proceed once we hear from you again labels Apr 3, 2018
@gunnarbeutner
Copy link
Contributor Author

I guess I might reword this to drop all the docs changes (apart from the bit where we mention that users can optionally add var in their loop var decls).

@gunnarbeutner
Copy link
Contributor Author

I've updated the PR to remove most of the controversial doc and example config changes. 🙂

@gunnarbeutner
Copy link
Contributor Author

Any objections to setting the target to 2.10?

@dnsmichi dnsmichi added this to the 2.10.0 milestone Jul 3, 2018
@dnsmichi dnsmichi added enhancement New feature or request and removed TBD To be defined - We aren't certain about this yet labels Jul 3, 2018
@dnsmichi
Copy link
Contributor

dnsmichi commented Jul 3, 2018

Looks good, thanks :) Review/merge happens after 2.9.

@dnsmichi dnsmichi merged commit e519d6b into Icinga:master Jul 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration DSL, parser, compiler, error handling enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants