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

feat!: standardize workflow job parameters #176

Merged
merged 4 commits into from
Mar 6, 2023

Conversation

KyleTryon
Copy link
Contributor

@KyleTryon KyleTryon commented Jan 19, 2023

WorkflowJobs no longer have specific properties for preSteps or postSteps, instead these are now expected to be passed as additional parameters within the parameters object.

The new syntax for using pre/postSteps:

  myWorkflow.addJob(job, {
    name: 'custom-name',
    preSteps: [helloWorld],
    postSteps: [helloWorld],
  });

This will produce the following output:

my-workflow:
  jobs:
    - my-job:
        name: custom-name
        pre-steps:
          - run: echo hello world
        post-steps:
          - run: echo hello world

Additionally, this PR resolves an issue I came across in testing that does not appear reported. Previously the when key for workflows appeared to be required and would output an undefined value. This key will no longer be added if undefined.

@ghost
Copy link

ghost commented Jan 19, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@Xavientois
Copy link

Still not working when I tested it with your changes: https://gist.github.com/Xavientois/7ea9388408896f2be515c0140ab22a1a

@codecov
Copy link

codecov bot commented Feb 9, 2023

Codecov Report

Patch coverage: 90.00% and project coverage change: -0.13 ⚠️

Comparison is base (898897c) 96.16% compared to head (8612849) 96.04%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #176      +/-   ##
==========================================
- Coverage   96.16%   96.04%   -0.13%     
==========================================
  Files          62       62              
  Lines         678      682       +4     
  Branches       69       72       +3     
==========================================
+ Hits          652      655       +3     
  Misses         11       11              
- Partials       15       16       +1     
Impacted Files Coverage Δ
src/lib/Components/Workflow/exports/WorkflowJob.ts 100.00% <ø> (ø)
...Components/Workflow/exports/WorkflowJobAbstract.ts 93.75% <83.33%> (-6.25%) ⬇️
src/lib/Components/Workflow/exports/Workflow.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@KyleTryon
Copy link
Contributor Author

Thank you @Xavientois I believe I understand the issue now, I am going to move to merge this PR for separate reasons but then open a new PR to address your issue more directly. I believe that some somewhat large changes might be needed to how we handle orbs currently. I think it could be improved with some breaking changes. I think one of the major issues we are seeing now is the way the orb manifest gets parsed at runtime, which leaves you with no type information (even though it is static). Sorry for the confusion here, the problem is a bit deeper than I initially realized.

I will make sure your comments here are put back into the original issue for tracking. I will then experiment a little with how maybe we could better work with orbs.

@KyleTryon KyleTryon changed the title feat!: fix workflow parameters feat!: standardize workflow job parameters Mar 6, 2023
@KyleTryon KyleTryon requested a review from a team March 6, 2023 19:38
Copy link

@brivu brivu left a comment

Choose a reason for hiding this comment

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

LGTM

@KyleTryon KyleTryon merged commit f3287a8 into main Mar 6, 2023
@KyleTryon KyleTryon deleted the feat/fix-workflow-parameters branch March 6, 2023 21:02
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