campaigns: allow partial publishing of changesets #13476

LawnGnome · 2020-08-30T21:17:40Z

This makes just enough changes to support the prototype in sourcegraph/src-cli#294. See that PR for more detail on what's going on.

mrnugget · 2020-09-28T09:46:13Z

go.mod

@@ -5,6 +5,7 @@ go 1.14
 require (
 	cloud.google.com/go/bigquery v1.6.0 // indirect
 	cloud.google.com/go/pubsub v1.3.1
+	github.com/LawnGnome/campaign-schema v0.0.0-20200928040853-342b51d40db7


Should we create github.com/sourcegraph/campaigns-schema?

Oh, I totally forgot to write this down last night, because I was rushing: assuming we go with this approach, I intend to move this to github.com/sourcegraph/campaignutils, by analogy with codeintelutils.

Sounds good to me.

mrnugget · 2020-09-28T09:47:00Z

internal/campaigns/types.go

@@ -1714,7 +1716,7 @@ func (cs *CampaignSpec) Clone() *CampaignSpec {
 // UnmarshalValidate unmarshals the RawSpec into Spec and validates it against
 // the CampaignSpec schema and does additional semantic validation.
 func (cs *CampaignSpec) UnmarshalValidate() error {
-	return unmarshalValidate(schema.CampaignSpecSchemaJSON, []byte(cs.RawSpec), &cs.Spec)
+	return unmarshalValidate(cschema.CampaignSpecJSON, []byte(cs.RawSpec), &cs.Spec)


Mental note: we could move unmarshalValidate, with its yaml/json stuff, into cschema.

Good catch. I'll create a follow up issue.

sqs · 2020-09-28T23:52:13Z

BTW, it would be nice if you could describe what this is in the PR title instead of just "RFC 228", for all the people who aren't familiar with that RFC. 😃

LawnGnome · 2020-09-28T23:53:38Z

BTW, it would be nice if you could describe what this is in the PR title instead of just "RFC 228", for all the people who aren't familiar with that RFC. smiley

Definitely, now it feels like this moving past being a PoC of an RFC option! I'll update this and the src-cli PR in a moment, since I'm doing some RFC gardening right now.

LawnGnome · 2020-09-30T01:13:34Z

Flipped to review-ready. As with src-cli, 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.

sourcegraph-bot · 2020-09-30T01:14:16Z

Notifying subscribers in CODENOTIFY files for diff af8941b...e86a527.

Notify	File(s)
@eseliger	doc/user/campaigns/campaign_spec_yaml_reference.md doc/user/campaigns/hello_world_campaign.md doc/user/campaigns/index.md enterprise/internal/campaigns/resolvers/resolver_test.go enterprise/internal/campaigns/store_campaign_specs_test.go internal/campaigns/campaign_spec.go

mrnugget

Sweet!

Please add docs! :)

At the very least the part about published in the YAML reference should be updated with examples etc. And in the best case, the new capability is also shown in the "Publishing changesets to the code host" section

mrnugget · 2020-09-30T09:22:25Z

enterprise/internal/campaigns/store_campaign_specs_test.go

@@ -28,7 +29,7 @@ func testStoreCampaignSpecs(t *testing.T, ctx context.Context, s *Store, _ repos
 						Commit: campaigns.CommitTemplate{
 							Message: "commit message",
 						},
-						Published: false,
+						Published: override.FromBool(false),


Just an idea I had when looking at this and thinking about the naming of the package and the function: how about something like overridable.NewBool(false)? Not saying it needs to be done in this PR or now, but override.FromBool makes me think "override from a bool? what's being overridden?"

And var published overridable.Bool also reads well :)

overridable is definitely better. I'll make that change.

The reason I didn't use New is because it's not a heap allocation. (Directly, anyway. What Go chooses to do behind the scenes is its business.) I don't have super strong feelings on this, but I was looking for a verb that described what was generally happening, and From felt as good as any (converting a bool to an overridable.Bool).

Does changing the package name (ie overridable.FromBool()) address that concern?

schema/campaign_spec.schema.json

Co-authored-by: Thorsten Ball <mrnugget@gmail.com>

LawnGnome · 2020-09-30T16:56:13Z

Please add docs! :)

I was going to do that in a separate PR, but actually, you're right. I'll update this PR.

LawnGnome · 2020-09-30T21:26:59Z

Docs added. PTAL!

sqs · 2020-09-30T21:40:10Z

CHANGELOG.md

@@ -16,6 +16,7 @@ All notable changes to Sourcegraph are documented in this file.
 - The new GraphQL API query field `namespaceByName(name: String!)` makes it easier to look up the user or organization with the given name. Previously callers needed to try looking up the user and organization separately.
 - Changesets created by campaigns will now include a link back to the campaign in their body text. [#14033](https://github.com/sourcegraph/sourcegraph/issues/14033)
 - Users can now preview commits that are going to be created in their repositories in the campaign preview UI. [#14181](https://github.com/sourcegraph/sourcegraph/pull/14181)
+- The `published` flag in campaign specs [may now be an array](https://docs.sourcegraph.com/@main/user/campaigns/campaign_spec_yaml_reference#publishing-only-specific-changesets), which allows only specific changesets within a campaign to be published based on the repository name. [#13476](https://github.com/sourcegraph/sourcegraph/pull/13476)


Recommend starting with the more important bit:

You can now publish a subset of changesets in a campaign ...

Done, although I went with slightly different wording to better fit the house style of the changelog (which doesn't usually address entries to a reader):

A subset of changesets can now be published by setting the published flag in campaign specs to an array, which allows only specific changesets within a campaign to be published based on the repository name. #13476

doc/user/campaigns/campaign_spec_yaml_reference.md

sqs

Minor doc changes, otherwise lgtm. Thanks for writing docs!

mrnugget

Nice docs! Thank you!

(btw. you should go nuts with this in #progress and also record video/gif/whathaveyou)

mrnugget · 2020-10-01T04:38:47Z

doc/user/campaigns/index.md

@@ -130,9 +130,11 @@ name: hello-world

 changesetTemplate:
  # ...
-  published: false
+  published: true


What. the. hell. How did I manage to get that wrong? 😄

…-only-except

chrispine · 2020-10-01T20:55:41Z

doc/user/campaigns/campaign_spec_yaml_reference.md

+If no entries match, then the repository will not be published. To make the default true, add a wildcard entry as the last item in the array:
+
+```yaml
+published:
+  - github.com/*: false
+  - "*": true
+```


Doesn't this just mean "published: true"? Wouldn't the wildcard "*" need to be the first entry to set the default to true?

I'm sure I don't know what you're talking about.

LawnGnome added the batch-changes Issues related to Batch Changes label Aug 30, 2020

LawnGnome mentioned this pull request Aug 30, 2020

campaigns: allow partial publishing of changesets sourcegraph/src-cli#294

Merged

LawnGnome force-pushed the aharvey/template-only-except branch from 9963f38 to 2769aa5 Compare September 15, 2020 04:54

chrispine added this to the 3.21 milestone Sep 16, 2020

chrispine added the planned/3.20 label Sep 16, 2020

This was referenced Sep 16, 2020

Campaigns Sprint 1 Tracking issue #13828

Closed

Campaigns 3.20 Tracking issue #13068

Closed

LawnGnome removed this from the 3.21 milestone Sep 16, 2020

chrispine added this to the Campaigns Sprint 1 milestone Sep 16, 2020

LawnGnome removed this from the Campaigns Sprint 1 milestone Sep 17, 2020

LawnGnome force-pushed the aharvey/template-only-except branch from 2769aa5 to 21632e0 Compare September 28, 2020 04:26

LawnGnome linked an issue Sep 28, 2020 that may be closed by this pull request

Allow users to publish only a subset of changesets #13139

Closed

mrnugget approved these changes Sep 28, 2020

View reviewed changes

This was referenced Sep 28, 2020

campaigns: move unmarshalValidate, with its yaml/json stuff, into cschema #14249

Closed

campaigns: prototype changesetTemplate specialisation #13394

Closed

LawnGnome changed the title ~~campaigns: support RFC 228 prototype~~ campaigns: implement support for applying changeset template fields to specific repos Sep 29, 2020

eseliger approved these changes Sep 29, 2020

View reviewed changes

LawnGnome changed the title ~~campaigns: implement support for applying changeset template fields to specific repos~~ campaigns: allow partial publishing of changesets Sep 30, 2020

campaigns: allow partial changeset publishing

f52f466

LawnGnome force-pushed the aharvey/template-only-except branch from e98fd2d to f52f466 Compare September 30, 2020 00:52

Update to the latest campaignutils.

185390a

LawnGnome requested review from mrnugget and eseliger September 30, 2020 01:13

LawnGnome marked this pull request as ready for review September 30, 2020 01:13

mrnugget approved these changes Sep 30, 2020

View reviewed changes

eseliger approved these changes Sep 30, 2020

View reviewed changes

Update schema/campaign_spec.schema.json

c0b8ed2

Co-authored-by: Thorsten Ball <mrnugget@gmail.com>

LawnGnome added 3 commits September 30, 2020 10:00

Update generated file.

c25578a

Rename override to overridable.

58cad1a

Add docs.

5fc7817

sqs reviewed Sep 30, 2020

View reviewed changes

doc/user/campaigns/campaign_spec_yaml_reference.md Outdated Show resolved Hide resolved

sqs approved these changes Sep 30, 2020

View reviewed changes

LawnGnome added 3 commits September 30, 2020 14:51

Tweak CHANGELOG.

25acbaf

Lead with a non-glob example.

71d24ed

Fix docsite check warnings.

925c783

mrnugget approved these changes Oct 1, 2020

View reviewed changes

LawnGnome added 2 commits October 1, 2020 11:03

Merge remote-tracking branch 'sourcegraph/main' into aharvey/template…

5bd4438

…-only-except

Another go mod tidy run.

e86a527

LawnGnome merged commit 2a9e2b8 into main Oct 1, 2020

LawnGnome deleted the aharvey/template-only-except branch October 1, 2020 18:48

sourcegraph-bot mentioned this pull request Oct 1, 2020

Update Sourcegraph Docker insiders images sourcegraph/deploy-sourcegraph#998

Merged

1 task

chrispine reviewed Oct 1, 2020

View reviewed changes

chrispine mentioned this pull request Oct 13, 2020

CampaignSpecFields should include the importChangesets field #13009

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

campaigns: allow partial publishing of changesets #13476

campaigns: allow partial publishing of changesets #13476

LawnGnome commented Aug 30, 2020

mrnugget Sep 28, 2020

LawnGnome Sep 28, 2020

mrnugget Sep 28, 2020

mrnugget Sep 28, 2020

LawnGnome Sep 28, 2020

sqs commented Sep 28, 2020

LawnGnome commented Sep 28, 2020

LawnGnome commented Sep 30, 2020

sourcegraph-bot commented Sep 30, 2020 •

edited

Loading

mrnugget left a comment

mrnugget Sep 30, 2020

LawnGnome Sep 30, 2020

LawnGnome commented Sep 30, 2020

LawnGnome commented Sep 30, 2020

sqs Sep 30, 2020

LawnGnome Sep 30, 2020

sqs left a comment

mrnugget left a comment

mrnugget Oct 1, 2020

chrispine Oct 1, 2020

LawnGnome Oct 1, 2020

campaigns: allow partial publishing of changesets #13476

campaigns: allow partial publishing of changesets #13476

Conversation

LawnGnome commented Aug 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sqs commented Sep 28, 2020

LawnGnome commented Sep 28, 2020

LawnGnome commented Sep 30, 2020

sourcegraph-bot commented Sep 30, 2020 • edited Loading

mrnugget left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LawnGnome commented Sep 30, 2020

LawnGnome commented Sep 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sqs left a comment

Choose a reason for hiding this comment

mrnugget left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sourcegraph-bot commented Sep 30, 2020 •

edited

Loading