Skip to content

Introduce transformChanges to campaign spec (RFC 265)#398

Merged
mrnugget merged 21 commits into
mainfrom
mrn/transform-changes
Dec 11, 2020
Merged

Introduce transformChanges to campaign spec (RFC 265)#398
mrnugget merged 21 commits into
mainfrom
mrn/transform-changes

Conversation

@mrnugget

@mrnugget mrnugget commented Nov 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

This is the implementation of the suggested solution for Option B in RFC 265: Multiple changesets per repository to produce multiple changesets in a single repository.

It depends on https://github.com/sourcegraph/sourcegraph/pull/16235. It fixes https://github.com/sourcegraph/sourcegraph/issues/14970.

IMPORTANT: this only implements the first step in "Defintion of success". It does not yet implement addressing multiple changesets in the same repository separately (that's Part2 in the RFC).

What this does is to introduce the transformChanges property to campaign specs (see https://github.com/sourcegraph/sourcegraph/pull/16235 for schema change on server).

Here is an example campaign spec using it:

name: multi-changesets
description: This is a testing campaign that should have multiple changesets in a single repository.

on:
  - repositoriesMatchingQuery: repo:src-cli|automation-testing$

steps:
  - run: echo ".swp" >> .gitignore
    container: alpine:3
  - run: if [[ -e gopkg/main.go ]]; then echo "func unused() int { return 99 }" >> gopkg/main.go; fi
    container: alpine:3
  - run: mkdir foobar && echo "Hey there" foobar/hello.txt
    container: alpine:3

# Transform the changes produced in each repository.
transformChanges:
  # Group the file diffs by directory and produce one additional changeset per group.
  group:
    - directory: gopkg
      branch: thorsten/multi-changesets-backend # will replace the `branch` in the `changesetTemplate`
    - directory: foobar
      repository: github.com/sourcegraph/automation-testing # optional: only apply the rule in this repository
      branch: thorsten/multi-changesets-foobar # will replace the `branch` in the `changesetTemplate`

changesetTemplate:
  title: multi-changesets
  body: multi-changesets
  branch: thorsten/multi-changesets
  commit:
    message: multi-changesets
  published: false

Changelog of this PR

  • 02 Dec: Updated the code and the PR description to use the new branch property instead of branchSuffix to incorporate feedback on RFC 265.
  • 02 Dec: Added the optional repository attribute to the group.
  • 10 Dec: Added validation to make sure that multiple changesets cannot be created with the same branch in the same repository. Updated PR description.

This was referenced Nov 30, 2020
Merged
Closed
@mrnugget mrnugget force-pushed the mrn/transform-changes branch from a024399 to a52e9e9 Compare December 2, 2020 14:21
@mrnugget

mrnugget commented Dec 2, 2020

Copy link
Copy Markdown
Contributor Author

Just updated this to use the branch attribute in a group instead of branchSuffix based on the feedback in RFC 265.

@mrnugget mrnugget force-pushed the mrn/transform-changes branch from a52e9e9 to b635d03 Compare December 2, 2020 14:57
@mrnugget mrnugget force-pushed the mrn/transform-changes branch from ceb46fa to a5269a2 Compare December 10, 2020 09:40
@mrnugget mrnugget marked this pull request as ready for review December 10, 2020 14:19
@mrnugget mrnugget requested a review from a team December 10, 2020 14:21
@mrnugget

mrnugget commented Dec 10, 2020

Copy link
Copy Markdown
Contributor Author

@sourcegraph/campaigns @rrhyne This is now ready for review. Please see the PR description for what's included and what's not. Documentation and schema update in sourcegraph/sourcegraph are here: https://github.com/sourcegraph/sourcegraph/pull/16235

LawnGnome
LawnGnome approved these changes Dec 10, 2020
View reviewed changes

@LawnGnome LawnGnome left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread cmd/src/campaigns_common.go Outdated
Comment thread internal/campaigns/executor.go
Comment thread internal/campaigns/executor.go Outdated
chrispine
chrispine approved these changes Dec 11, 2020
View reviewed changes

@chrispine chrispine left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍🏻

mrnugget and others added 2 commits December 11, 2020 10:02
@mrnugget
Pluralize message correctly
fde7d30
@mrnugget @LawnGnome
Update internal/campaigns/executor.go
7a33dcf 
Co-authored-by: Adam Harvey <aharvey@sourcegraph.com>
@mrnugget mrnugget merged commit 6cccdc7 into main Dec 11, 2020
@mrnugget mrnugget deleted the mrn/transform-changes branch December 11, 2020 09:27
scjohns pushed a commit that referenced this pull request Apr 24, 2023
@mrnugget @LawnGnome
Introduce transformChanges to campaign spec (RFC 265) (#398)
a475226 
* Add transformChanges to campaign spec

* Update Executor integration test

* Add a failing test for code transformations

* Change execution cache to only cache diffs

* Rename from .patch to .diff

* Extract groupFileDiffs function

* Add test for GroupFileDiffs

* Add some comments

* Fix campaign progress printer for multiple changeset specs

* Display how many changeset specs were produced in one repo

* Add more tests for grouping changes

* Fix problems after rebase

* Switch from branchSuffix to branch

* Add a repository filter to the transformChanges.Group

* Check whether transformChanges is supported

* Validate that multiple changesets don't have same branch

* Add minLength to campaign spec

* Pluralize message correctly

* Update internal/campaigns/executor.go

Co-authored-by: Adam Harvey <aharvey@sourcegraph.com>

* Update feature date

* Add a changelog entry

Co-authored-by: Adam Harvey <aharvey@sourcegraph.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@LawnGnome LawnGnome LawnGnome approved these changes

@chrispine chrispine chrispine approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Allow to create multiple changeset specs in a single repository

3 participants

@mrnugget @LawnGnome @chrispine