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

campaigns: allow partial publishing of changesets #294

Merged
merged 4 commits into from
Oct 1, 2020

Conversation

LawnGnome
Copy link
Contributor

@LawnGnome LawnGnome commented Aug 30, 2020

This is a prototype of the option being presented in RFC 228. It needs to be used in conjunction with sourcegraph/sourcegraph#13476.

Note that this PR adds a dependency on https://github.com/LawnGnome/campaign-schema for schema-related code shared between this PR and sourcegraph/sourcegraph#13476; my intention is to move this to https://github.com/sourcegraph/campaignutils if accepted.

An example of the type of campaign you could run with this is:

name: hello-world
description: Add Hello World to READMEs

# Find all repositories that contain a README.md file.
on:
  - repositoriesMatchingQuery: file:README.md r:github.com/*
  - repositoriesMatchingQuery: file:README.md r:gitlab.sgdev.org

# In each repository, run this command. Each repository's resulting diff is captured.
steps:
  - run: echo Hello World | tee -a $(find -name README.md)
    container: alpine:3

# Describe the changeset (e.g., GitHub pull request) you want for each repository.
changesetTemplate:
  title:
    default: Hello World
    except:
      - github.com/sourcegraph/*: Hello Sourcegraph!
  body: My first campaign!
  branch: hello-world # Push the commit to this branch.
  commit:
    message: Append Hello World to all README.md files
  published:
    - github.com/LawnGnome/*: true

@mrnugget
Copy link
Contributor

NICE! I like this the most so far.

Two thoughts:

  1. The only: in the title: acts more like an overwrites: oder except: instead of an only:: "default value is 'Hello World', except when we match this, then it's "Hello Sourcegraph".
  2. The match in published.except: is not prefixed with match:. I wonder whether we could make that more uniform.

I feel like we're now 95% there!

@LawnGnome LawnGnome force-pushed the aharvey/template-only-except branch from 81e49c8 to 3c5a372 Compare September 15, 2020 04:55
@LawnGnome
Copy link
Contributor Author

This is roughly updated in line with the RFC and its discussion.

One thing we should talk about further is that I've separated out the schema and OverridableString into a new repo, which I've temporarily namespaced under my own user until we decide what we want to do. The reason for this is because we'll need a lot of this in sourcegraph/sourcegraph as well, so it makes sense to me to migrate the schema and common types into that repo. I'd still need to move OverridableBool into that repo, but it's late, and I think this is enough to get a general sense of where this would be going.

So, concerns?

@mrnugget
Copy link
Contributor

Few thoughts on the schema and OverridableString:

  1. Do we really need the whole functionality in sourcegraph/sourcegraph? I know that we need to share the raw JSON schema so we can do validation in the API layer, but while we currently do unmarshal the CampaignSpec into a Go struct, we don't really need to, right? We're currently only using the top-level fields, such as name, description. Considering this, I think only in src-cli would we need to expand and evaluate the globs in the campaign spec. (Of course, with server-side execution we would need all of that in sourcegraph/sourcegraph, so maybe you meant that)
  2. code intel related code in src-cli imports github.com/sourcegraph/codeintelutils — that would be our equivalent here, right?

@LawnGnome
Copy link
Contributor Author

  • Do we really need the whole functionality in sourcegraph/sourcegraph? I know that we need to share the raw JSON schema so we can do validation in the API layer, but while we currently do unmarshal the CampaignSpec into a Go struct, we don't really need to, right? We're currently only using the top-level fields, such as name, description. Considering this, I think only in src-cli would we need to expand and evaluate the globs in the campaign spec. (Of course, with server-side execution we would need all of that in sourcegraph/sourcegraph, so maybe you meant that)

We don't, but we will, as you said. I'm happy enough to cut this back and embed the logic in src-cli for now, but with server side execution the shared bits will need to be shared.

  • code intel related code in src-cli imports github.com/sourcegraph/codeintelutils — that would be our equivalent here, right?

Exactly.

@LawnGnome LawnGnome force-pushed the aharvey/template-only-except branch from 66343c3 to 7d1f8a6 Compare September 28, 2020 04:26
internal/campaigns/executor.go Outdated Show resolved Hide resolved
@LawnGnome LawnGnome changed the title campaigns: support overridable changesetTemplate properties campaigns: implement support for applying changeset template fields to specific repos Sep 29, 2020
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.

Very cool, so simple and straightforward!

@LawnGnome LawnGnome force-pushed the aharvey/template-only-except branch from 7d1f8a6 to 58286e6 Compare September 30, 2020 00:49
@LawnGnome LawnGnome changed the title campaigns: implement support for applying changeset template fields to specific repos campaigns: allow partial publishing of changesets Sep 30, 2020
@LawnGnome LawnGnome marked this pull request as ready for review September 30, 2020 01:11
@LawnGnome
Copy link
Contributor Author

Flipped to review-ready. I'm going to dismiss the existing approvals, but I don't think there's anything controversial here.

The original branch that supported strings is pushed to the aharvey/override-strings branch.

@LawnGnome LawnGnome merged commit 173ba5e into main Oct 1, 2020
@LawnGnome LawnGnome deleted the aharvey/template-only-except branch October 1, 2020 18:48
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