This pipeline job has to use scripted syntax to set up CIBuildTrigger with properties build step. Jenkins forbids properties step in a declarative pipeline script and currently CIBuildTrigger can't be set in declarative pipeline's triggers block.

@lholecek @mikeb @csomh @ralph Hi, could you do a review?

@mikeb I am planning to move the CIBuildTrigger setup snippet into c3i-library. Do you think it is worth it? If so, where do you recommend to place the function?

This is confusing indentation. Can you indent one more level?

Is required: true needed since you provide default value?

Is required: true needed since you provide default value?

Good question. I actually tried removing required: true from those fields with default values. It turned out that a user can set those fields to empty string out of spite to produce invalid output.

This is confusing indentation. Can you indent one more level?

Sure.

Is required: true needed since you provide default value?

Good question. I actually tried removing required: true from those fields with default values. It turned out that a user can set those fields to empty string out of spite to produce invalid output.

I'm not adamant about it, but leave the required: out. If user sets the variable to empty string, it's probably intended (if it's a problem it'll be caught elsewhere).

Actually, I would prefer to leave out any field that is not necessary. Reviewing these k8s config files and Jenkinsfiles is not easy for me (I'm still not very qualified to do these reviews).

@lholecek Thanks. Updated.

https://pagure.io/waiverdb.git?

Trying to figure out what this means. Could only find it in source code:

Schedule a new job for every triggering message.

I assume this describes behavior when noSquash=true, not when the default false value is used.

It's bit confusing description and option name. Can you add a comment or remove this line since it's default anyway?

This looks great! I think just WAIVERDB_GIT_REPO needs to be updated to point to the upstream repo instead of your fork. Otherwise, +1!

@lholecek @mikeb Fixed.

+1 Thanks!

@gnaponie Can you review and merge this?

Looks good. +1

Commit 0efe415a fixes this pull-request