[FIX JENKINS-41966] load and save using the editor from Github #886
Conversation
pom.xml
Outdated
| @@ -133,49 +133,49 @@ | |||
| <dependency> | |||
| <groupId>${project.groupId}</groupId> | |||
| <artifactId>blueocean-commons</artifactId> | |||
| <version>${project.version}</version> | |||
| <version>${project.parent.version}</version> | |||
There was a problem hiding this comment.
oh? What difference does this make vs project?
There was a problem hiding this comment.
I should probably revert this, but the issue was if you use the blueocean parent (which I was trying for another reason) you have to set your plugin to the same as the blueocean verasion. But one shouldn't need to use the blueocean parent, so...
…-save-from-github
…-save-from-github
There was a problem hiding this comment.
Overall these changes LGTM.
I do have a question about the changes in RunDetails and background persist code. It's not clear to me what issue that change is trying to fix. Since I've worked on some bugs in that area just want to understand the changes to help determine whether they might regress anything.
| @@ -64,6 +64,7 @@ class RunDetails extends Component { | |||
|
|
|||
| componentWillMount() { | |||
| this._fetchRun(this.props, true); | |||
| this.opener = locationService.previous; | |||
There was a problem hiding this comment.
Good call, this is much simpler code. Can remove the true as second param up above though.
| router.goBack(); | ||
| } else { | ||
| const fallbackUrl = buildPipelineUrl(params.organization, params.pipeline); | ||
| router.push(fallbackUrl); |
There was a problem hiding this comment.
I am not sure if push or replace is more appropriate here.
There was a problem hiding this comment.
hmm, I'm not really sure, either. replace might be more consistent, but if i go to the page directly, and accidentally hit esc or something, it would be nice to go back in that case.
| const classListAsArray = new Array(classList.length); | ||
| for (let i = 0, len = classList.length; i < len; i++) { | ||
| classListAsArray[i] = classList[i]; | ||
| } |
There was a problem hiding this comment.
You may be able to simplify this code to:
const classListAsArray = [... classList]
There was a problem hiding this comment.
good call, updated.
|
@cliffmeyers the |
kzantow
changed the title
|
@michaelneale happy to have you give this a smoke test before merging, if you had some time - especially making sure loading indicators aren't acting really crazy, though you might see more, and the run details seems to work as expected wherever you navigate from |
| runnable={ this.props.pipeline } | ||
| latestRun={ currentRun } | ||
| buttonType="stop-only" | ||
| />, | ||
| <Extensions.Renderer extensionPoint="jenkins.blueocean.rundetails.top.widgets" |
|
@kzantow nice - yes I can see loading worm on creation too - which is a nice addition. Of course when it scrolls down it isnt visible but that doesn't matter. LGTM 🐝 This is a tour de force of necessary additions and improvements. |
Description
Only real changes here are:
blueocean-core-jsRunDetailsto use router historyCreate PipelinebuttonBefore the
pipeline-editorwork is completed and the plugin is installed (and we'll update the version in the POM), we'll get this for creation, which I think is acceptable:See JENKINS-41966.
Submitter checklist
Reviewer checklist