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

fixes concurrent build issue #1

Merged
merged 2 commits into from
Jun 5, 2018

Conversation

hrishin
Copy link
Collaborator

@hrishin hrishin commented Jun 3, 2018

  • changed the plugin code to manage build throttling configuration from "Manage Jenkins" (central) pane
  • added a jobs listener to add throttling behavior while creating new workflow jobs

Fixes openshiftio/openshift.io#2729

Copy link

@rohanKanojia rohanKanojia left a comment

Choose a reason for hiding this comment

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

lgtm


@Test
public void should_add_blocking_properties_from_config_file() throws IOException {
FreeStyleProject job1 = jenkins.createFreeStyleProject("testJob");

Choose a reason for hiding this comment

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

maybe I will call testJob instead of job1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

better, fixed it.

if(job.getProperty(BuildBlockerProperty.class) == null) {
try {
BuildBlockerProperty buildBlockerProperty = BuildBlockerProperty.getDefaultProperties();
LOG.log(Level.INFO, "Adding blocking properties "+buildBlockerProperty+" to job "+ job.getName());

Choose a reason for hiding this comment

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

Stupid thing, can you add white space between + of string concatenation? Just that is the de-facto format way :)

job.removeProperty(BuildBlockerProperty.class);
job.addProperty(property);
} catch (IOException e) {

Choose a reason for hiding this comment

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

should we add a log message or something? Just to be sure that we do not loose any important information.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@hrishin hrishin force-pushed the iss#2729-throttle branch from dcff920 to 7d3d109 Compare June 5, 2018 13:44
- changed the plugin code to manage build throttling configuration from 'Manage Jenkins Configuration' (central) pane
- added the job listener to add throttling behavior for new workflow jobs
- added unit tests to check build blocking properties
- updated README about plugin configuration for fabric8 jenkins deployment
- fixed review comments
@hrishin hrishin force-pushed the iss#2729-throttle branch from 7d3d109 to a8ae9b1 Compare June 5, 2018 16:22
@hrishin hrishin merged commit 6fa6af5 into fabric8-jenkins:master Jun 5, 2018
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.

3 participants