Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions enterprise/internal/campaigns/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,16 @@ func buildCommitOpts(repo *repos.Repo, spec *campaigns.ChangesetSpec) (protocol.
return opts, err
}

commitAuthorName, err := desc.AuthorName()
if err != nil {
return opts, err
}

commitAuthorEmail, err := desc.AuthorEmail()
if err != nil {
return opts, err
}

opts = protocol.CreateCommitFromPatchRequest{
Repo: api.RepoName(repo.Name),
BaseCommit: api.CommitID(desc.BaseRev),
Expand All @@ -356,8 +366,8 @@ func buildCommitOpts(repo *repos.Repo, spec *campaigns.ChangesetSpec) (protocol.

CommitInfo: protocol.PatchCommitInfo{
Message: commitMessage,
AuthorName: "Sourcegraph",
AuthorEmail: "campaigns@sourcegraph.com",
AuthorName: commitAuthorName,
AuthorEmail: commitAuthorEmail,
Comment on lines +369 to +370
Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed this behaviour in our sync today: we could patch in default values here if we wanted to retain compatibility with old src-cli versions. Given that users will likely need to upgrade src-cli in general for 3.20 campaigns anyway, we've decided not to do so, since that would leave a time bomb in the backend code that we'd have to remove at a later date anyway.

Date: spec.CreatedAt,
},
// We use unified diffs, not git diffs, which means they're missing the
Expand Down
2 changes: 1 addition & 1 deletion enterprise/internal/campaigns/testing/specs.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ index e5af166..d44c3fc 100644
"published": false,

"commits": [
{"message": "git commit message", "diff": %q}]
{"message": "git commit message", "diff": %q, "authorName": "Mary McButtons", "authorEmail": "mary@example.com"}]
}`

return fmt.Sprintf(tmpl, repo, baseRev, repo, diff)
Expand Down
30 changes: 28 additions & 2 deletions internal/campaigns/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1956,9 +1956,35 @@ func (d *ChangesetSpecDescription) CommitMessage() (string, error) {
return d.Commits[0].Message, nil
}

// AuthorName returns the author name of the first GitCommitDescription in Commits. If the
// ChangesetSpecDescription doesn't have Commits it returns ErrNoCommits.
//
// We currently only support a single commit in Commits. Once we support more,
// this method will need to be revisited.
func (d *ChangesetSpecDescription) AuthorName() (string, error) {
if len(d.Commits) == 0 {
return "", ErrNoCommits
}
return d.Commits[0].AuthorName, nil
}

// AuthorEmail returns the author email of the first GitCommitDescription in Commits. If the
// ChangesetSpecDescription doesn't have Commits it returns ErrNoCommits.
//
// We currently only support a single commit in Commits. Once we support more,
// this method will need to be revisited.
func (d *ChangesetSpecDescription) AuthorEmail() (string, error) {
if len(d.Commits) == 0 {
return "", ErrNoCommits
}
return d.Commits[0].AuthorEmail, nil
}

type GitCommitDescription struct {
Message string `json:"message,omitempty"`
Diff string `json:"diff,omitempty"`
Message string `json:"message,omitempty"`
Diff string `json:"diff,omitempty"`
AuthorName string `json:"authorName,omitempty"`
AuthorEmail string `json:"authorEmail,omitempty"`
}

// unmarshalValidate validates the input, which can be YAML or JSON, against
Expand Down
20 changes: 15 additions & 5 deletions internal/campaigns/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,9 @@ func TestChangesetSpecUnmarshalValidate(t *testing.T) {
"published": false,
"commits": [{
"message": "commit message",
"diff": "the diff"
"diff": "the diff",
"authorName": "Mary McButtons",
"authorEmail": "mary@example.com"
}]
}`,
},
Expand All @@ -1012,7 +1014,9 @@ func TestChangesetSpecUnmarshalValidate(t *testing.T) {
"title": "my title",
"published": false,
"commits": [{
"diff": "the diff"
"diff": "the diff",
"authorName": "Mary McButtons",
"authorEmail": "mary@example.com"
}]
}`,
err: "4 errors occurred:\n\t* Must validate one and only one schema (oneOf)\n\t* baseRev is required\n\t* body is required\n\t* commits.0: message is required\n\n",
Expand All @@ -1037,7 +1041,9 @@ func TestChangesetSpecUnmarshalValidate(t *testing.T) {
"published": false,
"commits": [{
"message": "commit message",
"diff": "the diff"
"diff": "the diff",
"authorName": "Mary McButtons",
"authorEmail": "mary@example.com"
}]
}`,
err: ErrHeadBaseMismatch.Error(),
Expand All @@ -1056,11 +1062,15 @@ func TestChangesetSpecUnmarshalValidate(t *testing.T) {
"commits": [
{
"message": "commit message",
"diff": "the diff"
"diff": "the diff",
"authorName": "Mary McButtons",
"authorEmail": "mary@example.com"
},
{
"message": "commit message2",
"diff": "the diff2"
"diff": "the diff2",
"authorName": "Mary McButtons",
"authorEmail": "mary@example.com"
}
]
}`,
Expand Down
5 changes: 5 additions & 0 deletions migrations/1528395716_add_git_commit_author.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
BEGIN;

-- Nothing needed, non-destructive operation

COMMIT;
11 changes: 11 additions & 0 deletions migrations/1528395716_add_git_commit_author.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
BEGIN;
-- Add default values for git commit author (name and email)
UPDATE changeset_specs
SET spec = spec || json_build_object(
'commits',
json_build_array(
spec->'commits'->0 || '{"authorName": "Sourcegraph", "authorEmail": "campaigns@sourcegraph.com"}'
)
)::jsonb
WHERE jsonb_array_length(spec->'commits') > 0;
COMMIT;
46 changes: 46 additions & 0 deletions migrations/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions schema/campaign_spec.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,24 @@
"message": {
"type": "string",
"description": "The Git commit message."
},
"author": {
"title": "GitCommitAuthor",
"type": "object",
"description": "The author of the Git commit.",
"additionalProperties": false,
"required": ["name", "email"],
"properties": {
"name": {
"type": "string",
"description": "The Git commit author name."
},
"email": {
"type": "string",
"format": "email",
"description": "The Git commit author email."
}
}
}
}
},
Expand Down
18 changes: 18 additions & 0 deletions schema/campaign_spec_stringdata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion schema/changeset_spec.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"type": "object",
"description": "The Git commit to create with the changes.",
"additionalProperties": false,
"required": ["message", "diff"],
"required": ["message", "diff", "authorName", "authorEmail"],
"properties": {
"message": {
"type": "string",
Expand All @@ -68,6 +68,15 @@
"diff": {
"type": "string",
"description": "The commit diff (in unified diff format)."
},
"authorName": {
"type": "string",
"description": "The Git commit author name."
},
"authorEmail": {
"type": "string",
"format": "email",
"description": "The Git commit author email."
}
}
}
Expand Down
11 changes: 10 additions & 1 deletion schema/changeset_spec_stringdata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions schema/schema.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.