Skip to content
Open
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
4 changes: 1 addition & 3 deletions .github/workflows/cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ jobs:
uses: goreleaser/goreleaser-action@v2
with:
version: latest
args: release --rm-dist
args: release --clean
env:
GITHUB_TOKEN: ${{ secrets.REPO_TOKEN }}
FURY_TOKEN: ${{ secrets.FURY_TOKEN }}
FURY_ORG: ${{ secrets.FURY_ORG }}
59 changes: 29 additions & 30 deletions .goreleaser.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,55 +34,54 @@ builds:
archives:
- name_template: '{{ .ProjectName }}_{{- if eq .Os "Darwin" }}macos_{{- else }}{{- tolower .Os }}_{{end}}{{- if eq .Arch "amd64" }}x86_64{{- else if eq .Arch "386" }}i386{{- else }}{{ .Arch }}{{ end }}{{ if .Arm }}v{{ .Arm }}{{ end }}'
format_overrides:
- goos: windows
format: zip
- goos: windows
format: zip
checksum:
name_template: 'checksums.txt'
name_template: "checksums.txt"
snapshot:
name_template: "{{ .Tag }}-next"
changelog:
sort: asc
filters:
exclude:
- '^docs:'
- '^test:'
- "^docs:"
- "^test:"
brews:
-
name: spr
- name: spr
repository:
owner: ejoffe
name: homebrew-tap
homepage: https://github.com/ejoffe/spr
owner: rohan-mehta
name: homebrew-spr-tap
homepage: https://github.com/rohan-mehta/spr
description: Stacked Pull Requests on GitHub
install: |
bin.install "git-spr"
bin.install "git-amend"
bin.install "spr_reword_helper"
license: "MIT"
nfpms:
- file_name_template: '{{ .ProjectName }}_{{ .Arch }}{{ if .Arm }}v{{ .Arm }}{{ end }}'
id: packages
homepage: https://github.com/ejoffe/spr
description: Stacked Pull Requests on GitHub
maintainer: Eitan <[email protected]>
license: MIT
formats:
- deb
- rpm
dependencies:
- git
recommends:
- golang
# nfpms:
# - file_name_template: '{{ .ProjectName }}_{{ .Arch }}{{ if .Arm }}v{{ .Arm }}{{ end }}'
# id: packages
# homepage: https://github.com/ejoffe/spr
# description: Stacked Pull Requests on GitHub
# maintainer: Eitan <[email protected]>
# license: MIT
# formats:
# - deb
# - rpm
# dependencies:
# - git
# recommends:
# - golang
#snapcrafts:
# - name_template: '{{ .ProjectName }}_{{ .Arch }}{{ if .Arm }}v{{ .Arm }}{{ end }}'
# summary: Stacked Pull Requests on GitHub
# description: |
# spr is a tool to easily run and automate stacked diff workflows on GitHub.
# confinement: classic
# publish: true
publishers:
- name: fury.io
ids:
- packages
dir: "{{ dir .ArtifactPath }}"
cmd: curl -F package=@{{ .ArtifactName }} https://{{ .Env.FURY_TOKEN }}@push.fury.io/{{ .Env.FURY_ORG }}/
# publishers:
# - name: fury.io
# ids:
# - packages
# dir: "{{ dir .ArtifactPath }}"
# cmd: curl -F package=@{{ .ArtifactName }} https://{{ .Env.FURY_TOKEN }}@push.fury.io/{{ .Env.FURY_ORG }}/
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@

bin:
goreleaser --snapshot --skip-publish --clean
goreleaser --snapshot --skip=publish --clean
Copy link
Author

Choose a reason for hiding this comment

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

The new version of goreleaser changed this flag (See #427)

Copy link
Contributor

Choose a reason for hiding this comment

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

See #444 for a standalone PR that addresses this. This PR has many unrelated YAML changes that make it hard to be properly reviewed IMHO.


57 changes: 54 additions & 3 deletions github/githubclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,8 +533,52 @@ func addManualMergeNotice(body string) string {
"Do not merge manually using the UI - doing so may have unexpected results.*"
}

func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface, pullRequests []*github.PullRequest, pr *github.PullRequest, commit git.Commit, prevCommit *git.Commit) {
// embedSprDescription finds or creates a section delimited by:
//
// <!-- SPR data start: please do NOT edit this section -->
// <!-- SPR data end -->
//
// Anything between those markers is overwritten by newSprContent.
// If the markers do not exist, a new section is appended.
func embedSprDescription(existingBody, newSpr string) string {
startMarker := "<!-- SPR data start: please do NOT edit this section -->"
endMarker := "<!-- SPR data end -->"

startIndex := strings.Index(existingBody, startMarker)
endIndex := strings.Index(existingBody, endMarker)

// If both markers exist, replace:
if startIndex >= 0 && endIndex > 0 {
endIndex += len(endMarker)
result := existingBody[:startIndex] +
startMarker + "\n" + newSpr + "\n" +
endMarker +
existingBody[endIndex:]
return strings.TrimRight(result, "\n")
}

// If missing, append markers
trimmed := strings.TrimRight(existingBody, "\n")
if trimmed == "" {
// empty body
result := startMarker + "\n" + newSpr + "\n" + endMarker
return strings.TrimRight(result, "\n")
}

// Non-empty body, add a blank line, then markers
result := trimmed + "\n\n" +
startMarker + "\n" + newSpr + "\n" + endMarker
return strings.TrimRight(result, "\n")
}

func (c *client) UpdatePullRequest(
ctx context.Context,
gitcmd git.GitInterface,
pullRequests []*github.PullRequest,
pr *github.PullRequest,
commit git.Commit,
prevCommit *git.Commit,
) {
if c.config.User.LogGitHubCalls {
fmt.Printf("> github update %d : %s\n", pr.Number, pr.Title)
}
Expand All @@ -548,23 +592,30 @@ func (c *client) UpdatePullRequest(ctx context.Context, gitcmd git.GitInterface,
Str("FromBranch", pr.FromBranch).Str("ToBranch", baseRefName).
Interface("PR", pr).Msg("UpdatePullRequest")

// Generate the new "spr" content that we want to embed in the PR body
body := formatBody(commit, pullRequests, c.config.Repo.ShowPrTitlesInStack)
if c.config.Repo.PRTemplatePath != "" {
pullRequestTemplate, err := readPRTemplate(gitcmd, c.config.Repo.PRTemplatePath)
if err != nil {
log.Fatal().Err(err).Msg("failed to read PR template")
}
body, err = insertBodyIntoPRTemplate(body, pullRequestTemplate, c.config.Repo, pr)
newBody, err := insertBodyIntoPRTemplate(body, pullRequestTemplate, c.config.Repo, pr)
if err != nil {
log.Fatal().Err(err).Msg("failed to insert body into PR template")
}
body = newBody
}

// Only modify the portion in the SPR markers, keep user edits outside them
existingBody := pr.Body
sprUpdatedBody := embedSprDescription(existingBody, body)

title := &commit.Subject

input := genclient.UpdatePullRequestInput{
PullRequestId: pr.ID,
Title: title,
Body: &body,
Body: &sprUpdatedBody,
}

if !pr.InQueue {
Expand Down
46 changes: 46 additions & 0 deletions github/githubclient/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -825,3 +825,49 @@ func TestInsertBodyIntoPRTemplateErrors(t *testing.T) {
})
}
}

func TestEmbedSprDescription(t *testing.T) {
existingBody := `# Some User Heading

User paragraph that should remain untouched.

<!-- SPR data start: please do NOT edit this section -->
old spr content
<!-- SPR data end -->

Another user paragraph that should remain untouched as well.
`

newSpr := `updated spr content with new data`

want := `# Some User Heading

User paragraph that should remain untouched.

<!-- SPR data start: please do NOT edit this section -->
updated spr content with new data
<!-- SPR data end -->

Another user paragraph that should remain untouched as well.`
got := embedSprDescription(existingBody, newSpr)
if got != want {
t.Fatalf("Unexpected embedSprDescription result.\nGot:\n`%s`\n\nWant:\n`%s`\n", got, want)
}

// Test if markers are missing, we append them
existingBodyNoMarkers := `# Some User Heading

No markers here.
`
wantNoMarkers := `# Some User Heading

No markers here.

<!-- SPR data start: please do NOT edit this section -->
updated spr content with new data
<!-- SPR data end -->`
gotNoMarkers := embedSprDescription(existingBodyNoMarkers, newSpr)
if gotNoMarkers != wantNoMarkers {
t.Fatalf("Unexpected embedSprDescription result when markers are missing.\nGot:\n%s\n\nWant:\n%s\n", gotNoMarkers, wantNoMarkers)
}
}
Comment on lines +829 to +873
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to use subtests to simplify the naming of variables and be able to focus more on the task than on the naming. I've added a sub-test which I think was missing: when there trimmed body is empty. I didn't run the tests but I hope they work as expected.

Suggested change
func TestEmbedSprDescription(t *testing.T) {
existingBody := `# Some User Heading
User paragraph that should remain untouched.
<!-- SPR data start: please do NOT edit this section -->
old spr content
<!-- SPR data end -->
Another user paragraph that should remain untouched as well.
`
newSpr := `updated spr content with new data`
want := `# Some User Heading
User paragraph that should remain untouched.
<!-- SPR data start: please do NOT edit this section -->
updated spr content with new data
<!-- SPR data end -->
Another user paragraph that should remain untouched as well.`
got := embedSprDescription(existingBody, newSpr)
if got != want {
t.Fatalf("Unexpected embedSprDescription result.\nGot:\n`%s`\n\nWant:\n`%s`\n", got, want)
}
// Test if markers are missing, we append them
existingBodyNoMarkers := `# Some User Heading
No markers here.
`
wantNoMarkers := `# Some User Heading
No markers here.
<!-- SPR data start: please do NOT edit this section -->
updated spr content with new data
<!-- SPR data end -->`
gotNoMarkers := embedSprDescription(existingBodyNoMarkers, newSpr)
if gotNoMarkers != wantNoMarkers {
t.Fatalf("Unexpected embedSprDescription result when markers are missing.\nGot:\n%s\n\nWant:\n%s\n", gotNoMarkers, wantNoMarkers)
}
}
func TestEmbedSprDescription(t *testing.T) {
newSpr := `updated spr content with new data`
t.Run("Update with existing spr content", func(t *testing.T) {
existingBody := `# Some User Heading
User paragraph that should remain untouched.
<!-- SPR data start: please do NOT edit this section -->
old spr content
<!-- SPR data end -->
Another user paragraph that should remain untouched as well.
`
want := `# Some User Heading
User paragraph that should remain untouched.
<!-- SPR data start: please do NOT edit this section -->
updated spr content with new data
<!-- SPR data end -->
Another user paragraph that should remain untouched as well.`
got := embedSprDescription(existingBody, newSpr)
if got != want {
t.Fatalf("\nGot:\n%s\n\nWant:\n%s\n", got, want)
}
})
t.Run("No spr markers but body", func(t *testing.T) {
// Test if markers are missing, we append them
existingBody := `# Some User Heading
No markers here.
`
want := `# Some User Heading
No markers here.
<!-- SPR data start: please do NOT edit this section -->
updated spr content with new data
<!-- SPR data end -->`
got := embedSprDescription(existingBody, newSpr)
if got != want {
t.Fatalf("\nGot:\n%s\n\nWant:\n%s\n", got, want)
}
})
t.Run("No spr markers and empty body", func(t *testing.T) {
// Test if markers are missing, we append them
existingBody := " "
want := `
<!-- SPR data start: please do NOT edit this section -->
updated spr content with new data
<!-- SPR data end -->`
got := embedSprDescription(existingBody, newSpr)
if got != want {
t.Fatalf("\nGot:\n%s\n\nWant:\n%s\n", got, want)
}
})
}

Loading