CircleCI-Public / circleci-config-sdk-ts

Generate CircleCI Configuration YAML from JavaScript or TypeScript. Use Dynamic Configuration and the Config SDK together for live generative config.
https://circleci-public.github.io/circleci-config-sdk-ts/
Apache License 2.0
82 stars 29 forks source link

feat!: standardize workflow job parameters #176

Closed KyleTryon closed 1 year ago

KyleTryon commented 1 year ago

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 commented 1 year ago
👇 Click on the image for a new way to code review #### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/CircleCI-Public/circleci-config-sdk-ts/176/7ad590ae/8612849cc0f27a3021d6558fc02410b2a5b7c7aa.svg)](https://app.codesee.io/r/reviews?pr=176&src=https%3A%2F%2Fgithub.com%2FCircleCI-Public%2Fcircleci-config-sdk-ts) #### Legend CodeSee Map legend
Xavientois commented 1 year ago

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

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 90.00% and project coverage change: -0.13 :warning:

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

Additional details and impacted files ```diff @@ 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](https://codecov.io/gh/CircleCI-Public/circleci-config-sdk-ts/pull/176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CircleCI-Public) | Coverage Δ | | |---|---|---| | [src/lib/Components/Workflow/exports/WorkflowJob.ts](https://codecov.io/gh/CircleCI-Public/circleci-config-sdk-ts/pull/176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CircleCI-Public#diff-c3JjL2xpYi9Db21wb25lbnRzL1dvcmtmbG93L2V4cG9ydHMvV29ya2Zsb3dKb2IudHM=) | `100.00% <ø> (ø)` | | | [...Components/Workflow/exports/WorkflowJobAbstract.ts](https://codecov.io/gh/CircleCI-Public/circleci-config-sdk-ts/pull/176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CircleCI-Public#diff-c3JjL2xpYi9Db21wb25lbnRzL1dvcmtmbG93L2V4cG9ydHMvV29ya2Zsb3dKb2JBYnN0cmFjdC50cw==) | `93.75% <83.33%> (-6.25%)` | :arrow_down: | | [src/lib/Components/Workflow/exports/Workflow.ts](https://codecov.io/gh/CircleCI-Public/circleci-config-sdk-ts/pull/176?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CircleCI-Public#diff-c3JjL2xpYi9Db21wb25lbnRzL1dvcmtmbG93L2V4cG9ydHMvV29ya2Zsb3cudHM=) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CircleCI-Public). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CircleCI-Public)

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

KyleTryon commented 1 year ago

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.