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

[JENKINS-29922] Describe step call formats in “Pipeline Syntax: Reference” #35

Merged
merged 3 commits into from Jul 28, 2016

Conversation

Projects
None yet
4 participants
@jglick
Copy link
Member

jglick commented Jul 28, 2016

Follow-up to #28.

@reviewbybees esp. @kohsuke, @abayer

@reviewbybees

This comment has been minimized.

Copy link

reviewbybees commented Jul 28, 2016

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

<li>
<p>
As maps of parameters. Default values may be omitted.
(Note that Groovy map literal syntax resembles list syntax in using square brackets, but keys and values are separated by colons.)

This comment has been minimized.

Copy link
@kohsuke

kohsuke Jul 28, 2016

Member

Syntax example would be useful for the completeness sake

This comment has been minimized.

Copy link
@jglick

jglick Jul 28, 2016

Author Member

But the example does show both map and list literals.

(Note that Groovy map literal syntax resembles list syntax in using square brackets, but keys and values are separated by colons.)
</p>
<p>
The special map key <code>$class</code> is used to represent the simple or (where necessary) fully-qualified class name

This comment has been minimized.

Copy link
@kohsuke

kohsuke Jul 28, 2016

Member

I think we should explain the better syntax first and position this as a fallback.

This comment has been minimized.

Copy link
@jglick

jglick Jul 28, 2016

Author Member

That was my first thought, but it did not work out well, since the newer syntax is of more limited applicability, and it is necessary to refer to concepts which are better explained with the earlier syntax.

</li>
<li>
<p>
Jenkins core 2.2+ and some newer plugins define custom <em>symbols</em> for many configuration objects.

This comment has been minimized.

Copy link
@kohsuke

kohsuke Jul 28, 2016

Member

Version information is a minor detail that should be at the end, not at the top of the pitch.

The nested configuration is an actual value which can be saved into variables etc.:
</p>
<pre>def archiverConfig(fingerprinting) {
fingerprinting ? archiveArtifacts(artifacts: '**.txt', fingerprint: true) : archiveArtifacts('**.txt')

This comment has been minimized.

Copy link
@kohsuke

kohsuke Jul 28, 2016

Member

This is actually incorrect as archiveArtifacts gets immediately executed via the meta step, and there's no way to avoid that.

This comment has been minimized.

Copy link
@jglick

jglick Jul 28, 2016

Author Member

Mm OK, so I need a different example.

Note that unlike with <code>$class</code> syntax, the symbol name must be specified even for elements of a homogeneous list.
</p>
<p>
The <b>Snippet Generator</b> tool will always produce this newer syntax when possible,

This comment has been minimized.

Copy link
@kohsuke

kohsuke Jul 28, 2016

Member

Perhaps I misunderstand the context in which this help is used, but I'd think this message should be at close to the top, before we start going into encyclopedic details of this syntax.

This comment has been minimized.

Copy link
@jglick

jglick Jul 28, 2016

Author Member

Which message? That Snippet Generator prefers this syntax? Not sure how you would phrase that without actually explaining what the syntax is first.

This comment has been minimized.

Copy link
@abayer

abayer Jul 28, 2016

Member

Yeah, I'd prefer the deep dark details like that be lower in the doc. I'm good with this.

@abayer

This comment has been minimized.

Copy link
Member

abayer commented Jul 28, 2016

🐝

@abayer

This comment has been minimized.

Copy link
Member

abayer commented Jul 28, 2016

re-🐝

Some configuration objects define custom <em>symbols</em>.
These are used with a notation looking like a step call, or other function call taking a map of named parameters:
</p>
<pre>step archiveArtifacts('**.txt')</pre>

This comment has been minimized.

Copy link
@kohsuke

kohsuke Jul 28, 2016

Member

This doesn't actually work like you wrote. at the end of archiveArtifacts evaluation, it always gets executed with a meta-step. The evaluation of that method doesn't know that there's a surrounding step function.

Is this indicative of a problem in the current code? I understand why people would expect the behaviour you wrote.

This comment has been minimized.

Copy link
@kohsuke

kohsuke Jul 28, 2016

Member

Here is one way to fix this. Normal execution environment doesn't allow archiveArtifacts call being invoked here to tell apart whether there's a function call around it or not (step in this case), but since we run this in our own interpreter that is groovy-cps, we can actually tell it apart.

By using the call site tagging mechanism in cloudbees/groovy-cps#36, we can mark those call sites that are at the statement level (aka top-level call), and change the behaviour of meta-step handling accordingly.

This comment has been minimized.

Copy link
@jglick

jglick Jul 28, 2016

Author Member

Too magical. I will just change the documentation.

<pre>def parallelism(deterministic) {
deterministic ? count(3) : time(15)
}
splitTests parallelism(true)</pre>

This comment has been minimized.

Copy link
@kohsuke
@kohsuke reminds me that symbols for valid metastep delegates can onl…
…y be used to actually run those steps, not as a part of a larger configuration.
@kohsuke

This comment has been minimized.

Copy link
Member

kohsuke commented Jul 28, 2016

🐝

@abayer

This comment has been minimized.

Copy link
Member

abayer commented Jul 28, 2016

re-re-🐝 =)

@jglick jglick merged commit 70fbe86 into jenkinsci:master Jul 28, 2016

1 check passed

Jenkins This pull request looks good
Details

@jglick jglick deleted the jglick:syntax-help-JENKINS-29922 branch Jul 28, 2016

svanoort pushed a commit that referenced this pull request Jun 5, 2017

Merge pull request #35 from iwarapter/master
[JENKINS-37327] allow empty stash.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.