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

Pipeline Declarative: Notifications #642

Conversation

bitwiseman
Copy link
Contributor

@bitwiseman bitwiseman commented Feb 14, 2017

@rtyler Draft complete.
@jenkins-infra/copy-editors - It is late, I'm sure there's plenty to be fixed here.

}
----

.Declarative is still Jenkins Pipeline
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rtyler -
This is meant as an aside,
In the process of writing theses blogs I feel like I've finally figured out a perspective on Declarative the makes sense to me and I wan to communicate that here. Technically, I could just drop this entire section - but I wrote and then deleted a similar section for the previous post, which indicate to me that it is worth bringing up.

I understand we're under time pressure, so if this seems off in the weeds, feel free to nuke it. I needed to finish it, that's all.

Copy link
Contributor

@daniel-beck daniel-beck left a comment

Choose a reason for hiding this comment

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

Needs fixing. Only got halfway through, just too much broken to properly review.


In this blog post, we'll pull in the notification method we created for
link:/blog/2016/07/18/pipline-notifications/[Sending Notifications in Pipeline].
We'll show how the structure of
Copy link
Contributor

Choose a reason for hiding this comment

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

Incomplete sentence.

In this blog post, we'll pull in the notification method we created for
link:/blog/2016/07/18/pipline-notifications/[Sending Notifications in Pipeline].
We'll show how the structure of
Then we'll move that notification sending method into shared library, letting
Copy link
Contributor

Choose a reason for hiding this comment

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

Grammar

link:/blog/2016/07/18/pipline-notifications/[Sending Notifications in Pipeline].
We'll show how the structure of
Then we'll move that notification sending method into shared library, letting
use call it from other projects and keeping our `Jenkinsfile` concise and
Copy link
Contributor

Choose a reason for hiding this comment

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

use -> us

- notifications
- slack
- hipchat
- emailext
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing a hyphen?

Copy link
Member

Choose a reason for hiding this comment

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

Nah, we should avoid adding hyphens to tags

Copy link
Contributor Author

@bitwiseman bitwiseman Feb 14, 2017

Choose a reason for hiding this comment

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

Same as the other blog post. Updating both.


== Setup

The setup for this post is a continuation of the previous.
Copy link
Contributor

@daniel-beck daniel-beck Feb 14, 2017

Choose a reason for hiding this comment

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

Should link back to that blog post.

****
Jenkins Declarative Pipeline is still *Jenkins*, still *Pipeline*, and still *Groovy*.
Declarative Pipeline - everything inside
the `pipeline {}` block - is a domain-specific subset is Groovy, but outside that
Copy link
Contributor

Choose a reason for hiding this comment

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

is Groovy -> of Groovy

****
Jenkins Declarative Pipeline is still *Jenkins*, still *Pipeline*, and still *Groovy*.
Declarative Pipeline - everything inside
the `pipeline {}` block - is a domain-specific subset is Groovy, but outside that
Copy link
Contributor

Choose a reason for hiding this comment

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

outside -> other than, besides?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Inside the block ... outside the block. I can change if you feel strongly about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Words, what do they mean?!

Jenkins Declarative Pipeline is still *Jenkins*, still *Pipeline*, and still *Groovy*.
Declarative Pipeline - everything inside
the `pipeline {}` block - is a domain-specific subset is Groovy, but outside that
we can still access all the facilities of Java, Groovy, and Scripted Pipeline.
Copy link
Contributor

Choose a reason for hiding this comment

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

ALL OF GROOVY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not understanding all caps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried a change. Better?

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant the same as before, Pipeline is a poor imitation of Groovy.

we can still access all the facilities of Java, Groovy, and Scripted Pipeline.

Looking at the method definition above, some of you will be saying,
"Wait minute, wasn't Declarative Pipeline was supposed to get me away from
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait a minute

we can still access all the facilities of Java, Groovy, and Scripted Pipeline.

Looking at the method definition above, some of you will be saying,
"Wait minute, wasn't Declarative Pipeline was supposed to get me away from
Copy link
Contributor

Choose a reason for hiding this comment

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

wasn't … was

@bitwiseman bitwiseman force-pushed the feature/declarative/notifications branch from 3b97689 to 2547055 Compare February 14, 2017 16:31
@bitwiseman
Copy link
Contributor Author

@daniel-beck Thanks for the feedback. I think I addressed everything except this "inside/outside" comment.

- notifications
- slack
- hipchat
- emailext
Copy link
Member

Choose a reason for hiding this comment

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

Nah, we should avoid adding hyphens to tags

In the
link:/blog/2017/02/10/declarative-html-publisher/[previous blog post],
we converted Scripted Pipeline to a Declarative Pipeline with descriptive stages
and `post` blocks. In one of those `post{ }` blocks, we included a placeholder for
Copy link
Member

Choose a reason for hiding this comment

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

post section, post section. Please be consistent with phrasing used in the syntax reference

@bitwiseman bitwiseman force-pushed the feature/declarative/notifications branch from 2547055 to f33fcb3 Compare February 14, 2017 19:52
Copy link
Member

@rtyler rtyler left a comment

Choose a reason for hiding this comment

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

of course s/pipeline/Pipeline/ where applicable again 😈

== Setup

The setup for this post is a almost the same as the
link:/blog/2017/02/10/declarative-html-publisher/[previous post].
Copy link
Member

Choose a reason for hiding this comment

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

"the previous post" is a bit vague, how about something like "my previous Declarative Pipeline post"


The setup for this post is a almost the same as the
link:/blog/2017/02/10/declarative-html-publisher/[previous post].
I've used a new branch
Copy link
Member

Choose a reason for hiding this comment

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

"I've used a new branch my same fork of the Hermann project:"

-->

"I've used a new branch in my fork of the Hermann project named:"

link:https://github.com/bitwiseman/hermann[my same fork] of the
link:https://github.com/reiseburo/hermann[hermann project]:
link:https://github.com/bitwiseman/hermann/tree/blog/declarative/notifications[`blog/add-declarative/notifications`].
I've set up a Multibranch Pipeline and pointed it at my repository
Copy link
Member

Choose a reason for hiding this comment

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

s/I've set up a/I have also set up a/

link:https://github.com/bitwiseman/hermann/tree/blog/declarative/notifications[`blog/add-declarative/notifications`].
I've set up a Multibranch Pipeline and pointed it at my repository
the same as did it previous post.
Also the same as before, I've set this Pipeline's Git configuration to
Copy link
Member

Choose a reason for hiding this comment

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

To avoid the repetition of "also, also, additionally" I suggest joining this last sentence with the sentence prior, e.g.

I have also set up a Multibranch Pipeline pointed at my repository, using the Pipeline's Git configuration to automatically "Clean after checkout", just as in my previous post.

Also the same as before, I've set this Pipeline's Git configuration to
automatically "Clean after checkout".

In addition, I still have my notification targets setup from the
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I follow what "notification targets" are in context here. Perhaps rephrase or elaborate on what this means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The channel on which I receive the notifications.

}
----

.vars/sendNotifications.txt
Copy link
Member

Choose a reason for hiding this comment

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

I suggest removing this file from the blog post, since it's not explained

* Send notifications based on build status string
*/
def call(String buildStatus = 'STARTED') {
// build status of null means successful
Copy link
Member

Choose a reason for hiding this comment

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

This is the third time in this blog post that the full method body has been pasted. I suggest just linking to the full file on GitHub so we can get to the Jenkinsfile faster here.

If you're married to including all this text (again!) then move the Jenkinsfile to the top.

----


== Conclusion
Copy link
Member

Choose a reason for hiding this comment

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

Before the inclusion, please include at least once screenshot of this "all working" with a notification showing up in HipChat or something like that. I need closure!

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'll to this in a minute.

end of the file with no reformatting elsewhere.
Then with the help of the Shared Library feature,
we've moved the implementation of `sendNotifications` out of
`Jenkinsfile`. This will let easily reuse that code in other
Copy link
Member

Choose a reason for hiding this comment

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

s/This will let easily/This will let us easily/

we've moved the implementation of `sendNotifications` out of
`Jenkinsfile`. This will let easily reuse that code in other
projects and maintains the clarity of our Pipeline.
In the next post, we'll look at running Sauce OnDemand with
Copy link
Member

Choose a reason for hiding this comment

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

This last sentence seems kind of crammed in here. I suggest rewording to flow better. (no suggestions off the top of my head)

@bitwiseman bitwiseman force-pushed the feature/declarative/notifications branch from 2ab21ca to 669b41b Compare February 14, 2017 21:21
@bitwiseman
Copy link
Contributor Author

@rtyler
re P-vs-p-ipeline: Dude, half the time it is debatable which meaning (and thus which capitalization applies. My first post I capitalized most them because I meant Pipeline, but the feed back was somewhat at random to lowercase. 😄 I'll review them again.

NOTE: This is a guest post by link:https://github.com/bitwiseman[Liam Newman],
Technical Evangelist at link:https://cloudbees.com[CloudBees].

**Declare Your Pipelines!**
Copy link
Contributor

Choose a reason for hiding this comment

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

Gentlemen, declare your pipelines! 😆

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, not a review finding.

integrating notification services Slack, HipChat, and Email, into our Declarative Pipeline.
Then we'll move that method into a shared library,
keeping our `Jenkinsfile` concise and understandable,
and letting use call that method from other projects.
Copy link
Contributor

Choose a reason for hiding this comment

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

use -> us


The setup for this post is a almost the same as the
link:/blog/2017/02/10/declarative-html-publisher/[my previous Declarative Pipeline post].
I've used a new branch in
Copy link
Contributor

Choose a reason for hiding this comment

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

I use


We'll start from the same Pipeline we had at the end of the previous post.

It works quiet well, but only only has that final `always` directive that prints a message to the console log,
Copy link
Contributor

Choose a reason for hiding this comment

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

quite


We'll start from the same Pipeline we had at the end of the previous post.

It works quiet well, but only only has that final `always` directive that prints a message to the console log,
Copy link
Contributor

Choose a reason for hiding this comment

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

only only


There are a few limitations for methods in Shared Libraries, but they don't apply here.
The minimal set of dependencies for `sendNotifications` means we can once again
basically copy-and-paste the code across.
Copy link
Contributor

Choose a reason for hiding this comment

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

across who or what?

Copy link
Member

Choose a reason for hiding this comment

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

Huh, I wouldn't have thought that this phrasing is particularly idiomatic, but it's correct English. "Copy and paste the code across from the original Jenkinsfile" is the implied meaning.

The minimal set of dependencies for `sendNotifications` means we can once again
basically copy-and-paste the code across.
We'll check this change into a branch in the library, named
`blog/declarative/notifications` the same as my branch in the `hermann` repository.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a comma missing somewhere?

We'll check this change into a branch in the library, named
`blog/declarative/notifications` the same as my branch in the `hermann` repository.
This will let us make changes on the master branch later without breaking this example.
We'll use use the `@Library` directive to tell Jenkins to use that branch's version
Copy link
Contributor

Choose a reason for hiding this comment

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

use use

`blog/declarative/notifications` the same as my branch in the `hermann` repository.
This will let us make changes on the master branch later without breaking this example.
We'll use use the `@Library` directive to tell Jenkins to use that branch's version
of the library with this Pipeline.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unclear sentence.

}
// Scripted //
----
<1> The `_` here is intentional.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the underscore appear correctly in output? GitHub doesn't like it.

I've used a new branch in
link:https://github.com/bitwiseman/hermann[my fork] of the
link:https://github.com/reiseburo/hermann[Hermann project]:
link:https://github.com/bitwiseman/hermann/tree/blog/declarative/notifications[`blog/add-declarative/notifications`].
Copy link
Member

Choose a reason for hiding this comment

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

Minor: the anchor text should match the URL, i.e. add-declarative -> declarative


I still have my private notification targets (where we'll send notifications) which I created for the
"link:/blog/2016/07/18/pipline-notifications/[Sending Notifications in Pipeline]" blog post.
Take a look at that post to review how to I setup the
Copy link
Member

Choose a reason for hiding this comment

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

"how to I setup" -> "how I set up"

I'd already set up a Multibranch Pipeline and pointed it at my repository,
so the new branch will be picked up and built automatically.

I still have my private notification targets (where we'll send notifications) which I created for the
Copy link
Member

Choose a reason for hiding this comment

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

Not sure the word "private" is necessary here.

The shared library functionality has too many configuration options to cover in one post.
I've chosen to configure this library as a "Global Pipeline Library",
accessible from any project on my Jenkins master.
To setup a "Global Pipeline Library", navigated to "Manage Jenkins" -> "Configure System"
Copy link
Member

Choose a reason for hiding this comment

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

"setup" -> "set up"


There are a few limitations for methods in Shared Libraries, but they don't apply here.
The minimal set of dependencies for `sendNotifications` means we can once again
basically copy-and-paste the code across.
Copy link
Member

Choose a reason for hiding this comment

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

Huh, I wouldn't have thought that this phrasing is particularly idiomatic, but it's correct English. "Copy and paste the code across from the original Jenkinsfile" is the implied meaning.

----
<1> The `_` here is intentional.
link:https://en.wikipedia.org/wiki/Java_annotation[Java/Groovy Annotations]
such `@Library` must be applied to an element.
Copy link
Member

Choose a reason for hiding this comment

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

"such" -> "such as"


=== Moving the Code to the Library

Moving a method or class to a library involves creating a file with the name of our method,
Copy link
Member

Choose a reason for hiding this comment

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

A wee bit more detail on the shared lib structure and naming would be nice, e.g.:

creating a file in the vars directory with the name of our method — in this case vars/sendNotifications.groovy

@bitwiseman bitwiseman force-pushed the feature/declarative/notifications branch from 8fcf844 to 126bcdf Compare February 15, 2017 07:05
@bitwiseman bitwiseman force-pushed the feature/declarative/notifications branch from 126bcdf to d4d7cfb Compare February 15, 2017 08:05
@bitwiseman bitwiseman force-pushed the feature/declarative/notifications branch from a2913af to 7239fd3 Compare February 15, 2017 17:47
@bitwiseman bitwiseman force-pushed the feature/declarative/notifications branch from fbdae29 to 5708400 Compare February 15, 2017 17:56
@bitwiseman
Copy link
Contributor Author

90 comments and counting. 😞 I will do better.

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.

4 participants