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-26107] stage may now take a block #4

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
8 participants
@jglick
Copy link
Member

commented May 12, 2016

@jtnord

This comment has been minimized.

Copy link
Member

commented May 12, 2016

🐝

1 similar comment
@amuniz

This comment has been minimized.

Copy link
Member

commented May 12, 2016

🐝

@reviewbybees

This comment has been minimized.

Copy link

commented May 12, 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.

@kohsuke

This comment has been minimized.

Copy link
Member

commented May 23, 2016

🐝

@abayer

This comment has been minimized.

Copy link
Member

commented Jun 3, 2016

Quick question - will this now work within parallel?

@amuniz

This comment has been minimized.

Copy link
Member

commented Jun 3, 2016

Quick question - will this now work within parallel?

Well, more precise is to say that now you can use stages inside parallel as long as you use the block-scope version (so you don't use concurrency). The visualization of that stages in stage-view is still undefined as the order is not granted.

@jglick

This comment has been minimized.

Copy link
Member Author

commented Jun 6, 2016

The visualization of that stages in stage-view is still undefined

No, it is defined: the stage view currently ignores block-scoped stage altogether, which is the reason this is still open. The intention is to first make the view honor top-level block-scoped stages for feature parity, and later improve it to show substructure.

@witokondoria

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

wouldnt it make sense to merge this PR if the feature is complete (stages can take a block) and leave the stage-view full compatibility for a later stage (pun intended)?

@jglick

This comment has been minimized.

Copy link
Member Author

commented Jun 27, 2016

Well the problem is that releasing this change actively deprecates the original syntax and encourages users to switch to blocky stages, but then they will soon find that the visualization becomes useless. So I would rather leave this open until the stage view can at least render top-level blocks correctly (@svanoort is working on this).

@svanoort

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

... and stage view changes are currently dependent on completing the AdvancedVisitor implementation and tests in jenkinsci/workflow-api-plugin#6 & a minimal FlowChunkStorage implementation for StageView (to couple to the current API response objects).

(Which itself depends on final review and merge of jenkinsci/workflow-api-plugin#2)

@jglick I think we still need to define some sort of handling for pipeline flownodes that are between valid stages (where there are nodes between closed stage blocks), for example:

stage('part-one') {
  echo 'I am part of a valid stage'
}
echo 'I am not part of a stage'
stage('part-two') {
  echo 'I am part of a valid stage too!'
}

My assumption would be to either create some sort of anonymous stage with a unique ID (so they don't get combined together or have name collisions), or ignore those steps for stage purposes.

@amuniz

This comment has been minimized.

Copy link
Member

commented Jun 28, 2016

My assumption would be to either create some sort of anonymous stage with a unique ID (so they don't get combined together or have name collisions), or ignore those steps for stage purposes.

+1 to ignore them. If a step is not inside a stage it seems reasonable that the step is ignored by stage-view.

@jglick

This comment has been minimized.

Copy link
Member Author

commented Jun 28, 2016

Either ignore them, or simply implicitly the wrap the entire build in a virtual stage.

@jglick jglick referenced this pull request Jul 1, 2016

Merged

[JENKINS-27039] New step: milestone #1

4 of 4 tasks complete
@jglick

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2016

Refiling as #5.

@jglick jglick closed this Aug 17, 2016

@jglick jglick deleted the jglick:block-step-JENKINS-26107 branch Aug 17, 2016

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.