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

Add support for forks #88

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
8813da5
Add docs for working with forks.
ojarjur Aug 13, 2018
a3b4308
Add skeletons for the various `git appraise fork ...` commands
ojarjur Aug 14, 2018
b0afaf4
Change `fork add` to take the owner emails as a comma-separated list
ojarjur Aug 14, 2018
ad819ac
Shorten the name of the `fork add` owner flag to make it consistent w…
ojarjur Aug 14, 2018
e099dbb
Added API surface area for adding, removing, and getting the forks of…
ojarjur Aug 17, 2018
8ec98c8
Add the `--include-forks` flag to the `pull` subcommand
ojarjur Aug 22, 2018
60181fd
Move the forks collection methods to a dedicated package
ojarjur Sep 5, 2018
b447bc4
Fill in the fork commands code
ojarjur Sep 5, 2018
742bae7
Fill in the git-repository implementation for pulling and pushing forks
ojarjur Sep 6, 2018
bd7636e
Make the git operations on forks work when there are no remote forks
ojarjur Sep 6, 2018
9792e0a
Make the logic for checking ref existence more resilient
ojarjur Sep 8, 2018
640d8d5
Implement the logic to list forks
ojarjur Sep 14, 2018
05b7e0b
Fix formatting for forks summary output
ojarjur Sep 14, 2018
d6849eb
Add support for adding forks
ojarjur Sep 17, 2018
09e179c
Don't error out when pulling with forks
ojarjur Sep 17, 2018
bf2de09
Remove unused `ShowAll` method
ojarjur Sep 17, 2018
f8bd6ef
Make the docs for the excludeFork option more consistent
ojarjur Sep 17, 2018
1311191
Implement the logic for deleting forks
ojarjur Sep 18, 2018
fd8e9e5
Fix bugs in the logic to merge forks refs
ojarjur Sep 18, 2018
3c91046
Implement fetching from forks
ojarjur Sep 18, 2018
5d90571
Don't re-write trees that already exist in the repository
ojarjur Sep 18, 2018
30583a8
Simplify the repository.Tree API
ojarjur Sep 18, 2018
f6953ae
Support loading diffs from reviews in fork branches
ojarjur Sep 22, 2018
60bfa7e
Support reading review refs from remotes other than origin
ojarjur Sep 22, 2018
99321ed
Merge review requests and comments from forks
ojarjur Sep 23, 2018
2e2d4aa
Add an integration test for forks
ojarjur Oct 2, 2018
d7d8adb
Make the forks test a bit simpler and realistic
ojarjur Oct 2, 2018
f218638
Make the forks test run multiple pulls to test the behavior when ther…
ojarjur Oct 5, 2018
48a2064
Restructure the fork pulling code to be simpler and to perform less w…
ojarjur Oct 5, 2018
38beeb6
Check committer instead of author for reviews from forks
ojarjur Oct 8, 2018
8d0f4c5
Check comment locations before merging them from a fork
ojarjur Oct 10, 2018
0eb8db8
Merge branch 'master' into ojarjur/forks
ojarjur Nov 13, 2018
4f4956c
Make the repo method for pushing more general
ojarjur Nov 13, 2018
b071f2f
Make the addForkFlagSet name match the command
ojarjur Jan 3, 2019
ede713a
Integrate support for forks with the new GPG support
ojarjur Jan 4, 2019
ae7411b
Remove unnecessary network calls to list refs in a remote repository …
ojarjur Jan 5, 2019
d621be5
Move the logic for pulling from forks into the `fork` package
ojarjur Jan 5, 2019
170aefc
Merge branch 'master' into ojarjur/forks
ojarjur Apr 14, 2020
eca3ab8
Update github mirror action to not try to overwrite the checked-out b…
ojarjur Apr 14, 2020
6c07ca4
Merge branch 'master' into ojarjur/forks
ojarjur Jun 8, 2020
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
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ This design removes the need for any sort of server-side setup. As a result,
this tool can work with any git hosting provider, and the only setup required
is installing the client on your workstation.

Additionally, code reviews can be conducted across multiple hosting providers.
The list of forks of a repository is also stored in the repository as git
objects, allowing code reviews to be pulled from every registered fork.

## Installation

Assuming you have the [Go tools installed](https://golang.org/doc/install), run
Expand Down Expand Up @@ -80,8 +84,15 @@ Submitting the current review:

git appraise submit [--merge | --rebase]

Adding a fork:

git appraise fork add -o <contributor-email>[,<contributor-email>]* <name> <url>

A more detailed getting started doc is available [here](docs/tutorial.md).

Instructions for using `git-appraise` with multiple forks can be found
[here](docs/forks.md).

## Metadata

The code review data is stored in [git-notes](https://git-scm.com/docs/git-notes),
Expand Down
59 changes: 45 additions & 14 deletions commands/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@ limitations under the License.
package commands

import (
"fmt"

"github.com/google/git-appraise/repository"
)

const notesRefPattern = "refs/notes/devtools/*"
const archiveRefPattern = "refs/devtools/archives/*"
const commentFilename = "APPRAISE_COMMENT_EDITMSG"
const (
notesRefPattern = "refs/notes/devtools/*"
devtoolsRefPattern = "refs/devtools/*"
archiveRefPattern = "refs/devtools/archives/*"
commentFilename = "APPRAISE_COMMENT_EDITMSG"
)

// Command represents the definition of a single command.
type Command struct {
Expand All @@ -41,15 +46,41 @@ func (cmd *Command) Run(repo repository.Repo, args []string) error {

// CommandMap defines all of the available (sub)commands.
var CommandMap = map[string]*Command{
"abandon": abandonCmd,
"accept": acceptCmd,
"comment": commentCmd,
"list": listCmd,
"pull": pullCmd,
"push": pushCmd,
"rebase": rebaseCmd,
"reject": rejectCmd,
"request": requestCmd,
"show": showCmd,
"submit": submitCmd,
"abandon": abandonCmd,
"accept": acceptCmd,
"comment": commentCmd,
"fork add": addForkCmd,
"fork list": listForksCmd,
"fork remove": removeForkCmd,
"list": listCmd,
"pull": pullCmd,
"push": pushCmd,
"rebase": rebaseCmd,
"reject": rejectCmd,
"request": requestCmd,
"show": showCmd,
"submit": submitCmd,
}

// FindSubcommand parses the subcommand from the list of arguments.
//
// The args parameter is the list of command line args after the program name.
//
// The return result are the matching command (if found), whether or not the
// command was found, and the list of remaining command line arguments that
// followed the subcommand.
func FindSubcommand(args []string) (*Command, bool, []string) {
if len(args) < 1 {
subcommand, ok := CommandMap["list"]
return subcommand, ok, []string{}
}
subcommand, ok := CommandMap[args[0]]
if ok {
return subcommand, ok, args[1:]
}
if len(args) > 1 {
subcommand, ok := CommandMap[fmt.Sprintf("%s %s", args[0], args[1])]
return subcommand, ok, args[2:]
}
return nil, false, []string{}
}
50 changes: 50 additions & 0 deletions commands/commands_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copyright 2018 Google Inc. All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package commands

import (
"strings"
"testing"
)

func TestFindSubcommandBuiltins(t *testing.T) {
for name, cmd := range CommandMap {
additionalArg := "foo"
matchingArgs := append(strings.Split(name, " "), additionalArg)
subcommand, ok, remainingArgs := FindSubcommand(matchingArgs)
if !ok {
t.Errorf("Failed to find the built-in subcommand %q", name)
} else if subcommand != cmd {
t.Errorf("Return the wrong subcommand for %q", name)
} else if len(remainingArgs) != 1 || remainingArgs[0] != additionalArg {
t.Errorf("Failed to return the remaining arguments for %q", name)
}
}
}

func TestFindSubcommandEmpty(t *testing.T) {
subcommand, ok, remaining := FindSubcommand([]string{})
if !ok {
t.Fatalf("Failed to return a default subcommand")
}
if subcommand != CommandMap["list"] {
t.Fatalf("Failed to return `list` as the default subcommand")
}
if len(remaining) != 0 {
t.Fatalf("Unexpected remaining arguments for an empty command: %q", remaining)
}
}
111 changes: 111 additions & 0 deletions commands/fork.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
/*
Copyright 2018 Google Inc. All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package commands

import (
"errors"
"flag"
"fmt"
"strings"

"github.com/google/git-appraise/commands/output"
"github.com/google/git-appraise/fork"
"github.com/google/git-appraise/repository"
)

var (
addForkFlagSet = flag.NewFlagSet("fork add", flag.ExitOnError)
addForkOwners = addForkFlagSet.String("o", "", "Comma-separated list of owner email addresses")
)

// addFork updates the local git repository to include the specified fork.
func addFork(repo repository.Repo, args []string) error {
addForkFlagSet.Parse(args)
args = addForkFlagSet.Args()

var owners []string
if len(*addForkOwners) > 0 {
for _, owner := range strings.Split(*addForkOwners, ",") {
owners = append(owners, strings.TrimSpace(owner))
}
}
if len(args) < 2 {
return errors.New("The name and URL of the fork must be specified.")
}
if len(args) > 2 {
return errors.New("Only the name and URL of the fork may be specified.")
}
if len(owners) == 0 {
return errors.New("You must specify at least one owner.")
}
name := args[0]
url := args[1]
return fork.Add(repo, fork.New(name, url, owners))
}

// listForks lists the forks registered in the local git repository.
func listForks(repo repository.Repo, args []string) error {
forks, err := fork.List(repo)
if err != nil {
return err
}
output.PrintForks(forks)
return nil
}

// removeFork updates the local git repository to no longer include the specified fork.
func removeFork(repo repository.Repo, args []string) error {
if len(args) < 1 {
return errors.New("The name of the fork must be specified.")
}
if len(args) > 1 {
return errors.New("Only the name of the fork may be specified.")
}
name := args[0]
return fork.Delete(repo, name)
}

// addForkCmd defines the `fork add` command.
var addForkCmd = &Command{
Usage: func(arg0 string) {
fmt.Printf("Usage: %s fork add [<option>...] <name> <url>\n\nOptions:\n", arg0)
addForkFlagSet.PrintDefaults()
},
RunMethod: func(repo repository.Repo, args []string) error {
return addFork(repo, args)
},
}

// listForksCmd defines the `fork add` command.
var listForksCmd = &Command{
Usage: func(arg0 string) {
fmt.Printf("Usage: %s fork list\n", arg0)
},
RunMethod: func(repo repository.Repo, args []string) error {
return listForks(repo, args)
},
}

// removeForkCmd defines the `fork add` command.
var removeForkCmd = &Command{
Usage: func(arg0 string) {
fmt.Printf("Usage: %s fork remove <name>\n", arg0)
},
RunMethod: func(repo repository.Repo, args []string) error {
return removeFork(repo, args)
},
}
17 changes: 17 additions & 0 deletions commands/output/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"strings"
"time"

"github.com/google/git-appraise/fork"
"github.com/google/git-appraise/repository"
"github.com/google/git-appraise/review"
)
Expand Down Expand Up @@ -61,6 +62,14 @@ status: %s
`
// Number of lines of context to print for inline comments
contextLineCount = 5
// Template for printing the summary of the forks.
forksSummaryTemplate = `%d forks
`
// Template for printing the summary of a code review.
forkTemplate = `%q
owners: %q
urls: %q
`
)

// getStatusString returns a human friendly string encapsulating both the review's
Expand Down Expand Up @@ -256,3 +265,11 @@ func PrintDiff(r *review.Review, diffArgs ...string) error {
fmt.Println(diff)
return nil
}

// PrintForks prints the list of forks.
func PrintForks(forks []*fork.Fork) {
fmt.Printf(forksSummaryTemplate, len(forks))
for _, f := range forks {
fmt.Printf(forkTemplate, f.Name, f.Owners, f.URLS)
}
}
39 changes: 25 additions & 14 deletions commands/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"flag"
"fmt"

"github.com/google/git-appraise/fork"
"github.com/google/git-appraise/repository"
"github.com/google/git-appraise/review"
)
Expand All @@ -29,6 +30,7 @@ var (
pullFlagSet = flag.NewFlagSet("pull", flag.ExitOnError)
pullVerify = pullFlagSet.Bool("verify-signatures", false,
"verify the signatures of pulled reviews")
pullIncludeForks = pullFlagSet.Bool("include-forks", true, "Also pull reviews and comments from forks.")
)

// pull updates the local git-notes used for reviews with those from a remote
Expand All @@ -38,26 +40,28 @@ func pull(repo repository.Repo, args []string) error {
pullArgs := pullFlagSet.Args()

if len(pullArgs) > 1 {
return errors.New(
"Only pulling from one remote at a time is supported.")
return errors.New("Only pulling from one remote at a time is supported.")
}

remote := "origin"
if len(pullArgs) == 1 {
remote = pullArgs[0]
}
// This is the easy case. We're not checking signatures so just go the
// normal route.

if !*pullVerify && !*pullIncludeForks {
return repo.PullNotesAndArchive(remote, notesRefPattern, archiveRefPattern)
}
if !*pullVerify {
return repo.PullNotesAndArchive(remote, notesRefPattern,
archiveRefPattern)
if _, err := repo.PullNotesForksAndArchive(remote, notesRefPattern, fork.Ref, archiveRefPattern); err != nil {
return fmt.Errorf("failure pulling review metadata from the remote %q: %v", remote, err)
}
return fork.Pull(repo, *pullVerify)
}

// Otherwise, we collect the fetched reviewed revisions (their hashes), get
// We collect the fetched reviewed revisions (their hashes), get
// their reviews, and then one by one, verify them. If we make it through
// the set, _then_ we merge the remote reference into the local branch.
revisions, err := repo.FetchAndReturnNewReviewHashes(remote,
notesRefPattern, archiveRefPattern)
revisions, err := repo.FetchAndReturnNewReviewHashes(remote, notesRefPattern, archiveRefPattern)
if err != nil {
return err
}
Expand All @@ -74,17 +78,24 @@ func pull(repo repository.Repo, args []string) error {
}
fmt.Println("verified review:", revision)
}

err = repo.MergeNotes(remote, notesRefPattern)
if err != nil {
if err := repo.MergeNotes(remote, notesRefPattern); err != nil {
return err
}
if err := repo.MergeArchives(remote, archiveRefPattern); err != nil {
return err
}
if !*pullIncludeForks {
return nil
}
if err := repo.MergeForks(remote, fork.Ref); err != nil {
return err
}
return repo.MergeArchives(remote, archiveRefPattern)
return fork.Pull(repo, *pullVerify)
}

var pullCmd = &Command{
Usage: func(arg0 string) {
fmt.Printf("Usage: %s pull [<option>] [<remote>]\n\nOptions:\n", arg0)
fmt.Printf("Usage: %s pull [<option>...] [<remote>]\n\nOptions:\n", arg0)
pullFlagSet.PrintDefaults()
},
RunMethod: func(repo repository.Repo, args []string) error {
Expand Down
Loading