BREAKING CHANGE: refactor into libraries. add join flag. support multiple pr jobs #2
Conversation
lib/getNextJobs.js
Outdated
| /** | ||
| * Calculate the next jobs to execute, given a workflow and a trigger job | ||
| * @method getNextJobs | ||
| * @param {Object} workflow Directed graph representation of workflow |
There was a problem hiding this comment.
Can you rename this variable to workflowGraph so we don't get it mixed up with the old concept of workflow?
lib/getNextJobs.js
Outdated
| workflow.edges.forEach((edge) => { | ||
| if (edge.src === obj.trigger) { | ||
| // Make PR jobs PR-$num:$cloneJob (not sure how to better handle multiple PR jobs) | ||
| jobs.add(obj.trigger === '~pr' ? `PR-${obj.prNum}:${edge.dest}` : edge.dest); |
There was a problem hiding this comment.
I think we're going with PR-${obj.prNum}-${edge.dest}
There was a problem hiding this comment.
I think we should use : cuz jobName can actually contains/start with -.
lib/getWorkflow.js
Outdated
| if (useLegacy && !hasRequiresConfig) { | ||
| // Work out whether there is a user defined workflow, | ||
| // or if we use the order of jobs defined in jobConfig | ||
| const workflow = Array.isArray(pipelineConfig.workflow) ? |
There was a problem hiding this comment.
When it goes through the config parser, if will add main to the workflow. And workflow saved in the pipeline will always have that main job in workflow.
https://api.screwdriver.cd/v4/pipelines
Can we have some check for pipelineConfig.workflow? If it has main then take it out
ghost
assigned 
petey
changed the title
README.md
Outdated
| const { getWorkflow, getNextJobs } = require('screwdriver-workflow-parser'); | ||
|
|
||
| // Calculate the directed graph workflow from a pipeline config (and parse legacy workflows) | ||
| const workflowGraph = getWorkflow({ config: pipelineConfig, useLegacy: true }); |
lib/getNextJobs.js
Outdated
| * @param {String} [obj.prNum] The PR number (required when ~pr trigger) | ||
| * @return {Array} List of job names | ||
| */ | ||
| const getNextJobs = (workflowGraph, obj) => { |
There was a problem hiding this comment.
probably want to use "config" to be consistent
lib/getWorkflow.js
Outdated
| /** | ||
| * Given a pipeline config, return a directed graph configuration that describes the workflow | ||
| * @method getWorkflow | ||
| * @param {Object} pipeline A Pipeline Config |
There was a problem hiding this comment.
Need to update JSdocs to match new arg name (pipelineConfig)
BREAKING CHANGE: altered the signature for getWorkflow, getNextJobs
Context:
Planning ahead to add workflow cycle detection, but wanted to get api changes out sooner.
Objective: