Skip to content

Conversation

@mrnugget
Copy link
Contributor

@mrnugget mrnugget commented Jan 12, 2021

What?

This PR enables dynamic changesetTemplates in campaign specs by doing two things:

  1. it adds templating support to the title, body, branch and commit.message fields in changesetTemplate.
  2. it adds outputs to steps, which are values that are set after the step has been executed and that have access to template variables, including the current step's output.

This solves the problem of not being able to customize the changesetTemplate per repository (only published could be configured per repository).

Show me

With this PR, the following campaign spec works:

name: dynamic-changeset-template
description: Hello Outputs

on:
  - repository: github.com/sourcegraph/src-cli

steps:
  - run: echo "Hello there!" >> message.txt && cat message.txt
    container: alpine:3
    # NEW:
    outputs:
      friendlyMessage:
        value: "${{ step.stdout }}"
        # format: text is implicit
  - run: cat .goreleaser.yml >&2
    container: alpine:3
    # NEW:
    outputs:
      goreleaserConfig:
        value: "${{ step.stderr }}"
        # we will parse the value here as YAML, meaning that we can access it as an object
        format: yaml
      goreleaserConfigExists:
        # we can use the power of Go's text/template engine to dynamically produce complex values
        value: "exists: ${{ gt (len step.stderr) 0 }}"
        format: yaml

changesetTemplate:
  # title allows templating
  title: "The friendly message is: `${{ outputs.friendlyMessage }}`"
  # body allows templating
  body: |
    This repository has a `gorelaserConfig`: ${{ outputs.goreleaserConfigExists.exists }}.

    (Since these are just Go `text/templates` and `goreleaserConfig` was parsed as YAML we can access every field)

    The `goreleaser.yml` defines the following `before.hooks`:

    ${{ range $index, $hook := outputs.goreleaserConfig.before.hooks }}
    - `${{ $hook }}`
    ${{ end }}

    These files were added by the steps:

    ```
    ${{ join steps.added_files " " }}
    ```

  # branch allows templating
  branch: thorsten/variables-${{ outputs.goreleaserConfig.dist }}
  published: draft
  commit:
    # commit message allows templating
    message: "The friendly message is: `${{ outputs.friendlyMessage }}`"

It created this pull request:

screenshot_2021-01-14_15 38 19@2x

Meta

PR in sourcegraph/sourcegraph that adds the schema: https://github.com/sourcegraph/sourcegraph/pull/17243

What's missing?

Possible follow-ups

  • Can be a follow-up PR: making transformChanges.group.branch optional by making transform_changes.group available as a template variable in changesetTemplate.branch.

@mrnugget mrnugget force-pushed the mrn/dynamic-changeset-template branch from 42f3045 to 569e37d Compare January 13, 2021 13:08
@mrnugget mrnugget changed the title Spike for dynamic changeset template Enable dynamic changesetTemplates by adding templating & steps outputs Jan 13, 2021
@chrispine
Copy link

This is so cool! I'm thrilled to see this coming together!

@mrnugget mrnugget marked this pull request as ready for review January 14, 2021 14:40
@mrnugget mrnugget requested a review from a team January 14, 2021 14:40
@mrnugget
Copy link
Contributor Author

This is ready for review. I'll add the docs in a PR to sourcegraph/sourcegraph tomorrow, @sourcegraph/campaigns.

Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

N i c e, 🚢 it!!!11

Copy link
Contributor

@LawnGnome LawnGnome left a comment

Choose a reason for hiding this comment

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

I have a question around forward compatibility of cached changeset specs (which I think was probably already present, but is more obvious now), but otherwise this looks good! 👍

@mrnugget
Copy link
Contributor Author

@LawnGnome @eseliger After Adam spotted in bug in this comment I reworked the ExecutionDiskCache a bit so that

  1. it actually works and reads old cache files
  2. new: writes a new cache and deletes the old one if an old one was read
  3. new: handles multiple old cache files

I went a bit overboard with the tests, maybe, but they were a joy to write, so... reason enough to up the test coverage :)

Since we do have a branch cut later today, it would be great if @eseliger can review this and we merge this before then. @LawnGnome you can then also take another look and if there are issues we can follow up, but I want to cut a 3.24 release with this in it and bump the MiniumVersion in sourcegraph/sourcegraph before branch cut. We can always merge follow-up fixes and release a src-cli 3.24.1 then.

Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

:shipit:

@mrnugget
Copy link
Contributor Author

@LawnGnome We just reviewed this together, Erik and I. And Erik, of course, found something: the conversion from old caches to new caches is not lossless, since we don't recreate the steps.{modified,added,deleted,renamed}_files template var. Means in practice: if a user ran a campaign, populated caches, then thinks: "ahh, new template vars, let's add steps.added_files to my campaign spec and run again", then that wouldn't work.

@mrnugget mrnugget merged commit da41e6c into main Jan 15, 2021
@mrnugget mrnugget deleted the mrn/dynamic-changeset-template branch January 15, 2021 14:17
@LawnGnome
Copy link
Contributor

@LawnGnome you can then also take another look

I don't see anything problematic from where I sit, besides the cache lossiness issue Erik saw (which I'd be amazed if anyone ever hit). 👍

scjohns pushed a commit that referenced this pull request Apr 24, 2023
#424)

* Add templating to changesetTemplate and steps.outputs

* Fix parsing of outputs

* Remove leftovers in campaign spec schema

* Remove debug output

* Rename test function

* Add a test for renderChangesetTemplateField

* Update docs/examples in campaign spec schema

* Get dynamic changeset templates working with cache

* Simplify ExecutionCache by using ExecutionResult

* Change interface of runSteps to return ExecutionResult

* Add 'steps' to changesetTemplate template variables

* Support templating in changesetTemplate.author fields

* Fix doc comment

* Only use yaml.v3 in internal/campaigns

* Add tests for ExecutionCacheTest

* Add proper backwards-compatibility to ExecutionDiskCache

* Remove unneeded code

* Add changelog entry

* Add comment about lossiness of cache conversion
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.

5 participants