Merge branch 'trunk' into gh-gist-delete-prompt
This commit is contained in:
commit
7d1d261c82
16 changed files with 416 additions and 137 deletions
27
acceptance/testdata/pr/pr-create-without-upstream-config.txtar
vendored
Normal file
27
acceptance/testdata/pr/pr-create-without-upstream-config.txtar
vendored
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
# This test is the same as pr-create-basic, except that the git push doesn't include the -u argument
|
||||
# This causes a git config read to fail during gh pr create, but it should not be fatal
|
||||
|
||||
# Use gh as a credential helper
|
||||
exec gh auth setup-git
|
||||
|
||||
# Create a repository with a file so it has a default branch
|
||||
exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --add-readme --private
|
||||
|
||||
# Defer repo cleanup
|
||||
defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING
|
||||
|
||||
# Clone the repo
|
||||
exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING
|
||||
|
||||
# Prepare a branch to PR
|
||||
cd $SCRIPT_NAME-$RANDOM_STRING
|
||||
exec git checkout -b feature-branch
|
||||
exec git commit --allow-empty -m 'Empty Commit'
|
||||
exec git push origin feature-branch
|
||||
|
||||
# Create the PR
|
||||
exec gh pr create --title 'Feature Title' --body 'Feature Body'
|
||||
|
||||
# Check the PR is indeed created
|
||||
exec gh pr view
|
||||
stdout 'Feature Title'
|
||||
|
|
@ -1,27 +1,51 @@
|
|||
# Triage role
|
||||
|
||||
As we get more issues and pull requests opened on the GitHub CLI, we've decided on a weekly rotation
|
||||
triage role. The initial expectation is that the person in the role for the week spends no more than
|
||||
2 hours a day on this work; we can refine that as needed.
|
||||
As we get more issues and pull requests opened on the GitHub CLI, we've decided on a weekly rotation triage role as defined by our First Responder (FR) rotation. The primary responsibility of the FR during that week is to triage incoming issues from the Open Source community, as defined below. An issue is considered "triaged" when the `needs-triage` label is removed.
|
||||
|
||||
## Expectations for incoming issues
|
||||
## Expectations for triaging incoming issues
|
||||
|
||||
Review and label [open issues missing either the `enhancement`, `bug`, or `docs` label](https://github.com/cli/cli/issues?q=is%3Aopen+is%3Aissue+-label%3Abug%2Cenhancement%2Cdocs+).
|
||||
Review and label [open issues missing either the `enhancement`, `bug`, or `docs` label](https://github.com/cli/cli/issues?q=is%3Aopen+is%3Aissue+-label%3Abug%2Cenhancement%2Cdocs+) and the label(s) corresponding to the command space prefixed with `gh-`, such as `gh-pr` for the `gh pr` command set, or `gh-extension` for the `gh extension` command set.
|
||||
|
||||
Any issue that is accepted to be done as either an `enhancement`, `bug`, or `docs` requires explicit Acceptance Criteria in a comment on the issue before `needs-triage` label is removed.
|
||||
Then, engage with the issue and community with the goal to remove the `needs-triage` label from the issue. The heuristics for triaging the different issue types are as follow:
|
||||
|
||||
To be considered triaged, `enhancement` issues require at least one of the following additional labels:
|
||||
### Bugs
|
||||
|
||||
- `core`: reserved for the core CLI team
|
||||
- `help wanted`: signal that we are accepting contributions for this
|
||||
- `discuss`: add to our team's queue to discuss during a sync
|
||||
- `needs-investigation`: work that requires a mystery be solved by the core team before it can move forward
|
||||
- `needs-user-input`: we need more information from our users before the task can move forward
|
||||
To be considered triaged, `bug` issues require the following:
|
||||
|
||||
To be considered triaged, `bug` issues require a severity label: one of `p1`, `p2`, or `p3`, which are defined as follows:
|
||||
- `p1`: Affects a large population and inhibits work
|
||||
- `p2`: Affects more than a few users but doesn't prevent core functions
|
||||
- `p3`: Affects a small number of users or is largely cosmetic
|
||||
- A severity label `p1`, `p2`, and `p3`
|
||||
- Clearly defined Acceptance Criteria, added to the Issue as a standalone comment (see [example](https://github.com/cli/cli/issues/9469#issuecomment-2292315743))
|
||||
|
||||
#### Bug severities
|
||||
|
||||
| Severity | Description |
|
||||
| - | - |
|
||||
| `p1` | Affects a large population and inhibits work |
|
||||
| `p2` | Affects more than a few users but doesn't prevent core functions |
|
||||
| `p3` | Affects a small number of users or is largely cosmetic |
|
||||
|
||||
### Enhancements
|
||||
|
||||
To be considered triaged, `enhancement` issues require either
|
||||
|
||||
- Clearly defined Acceptance Criteria as above
|
||||
- The `needs-investigation` or `needs-design` label with a clearly defined set of open questions to be investigated.
|
||||
|
||||
### Docs
|
||||
|
||||
To be considered triaged, `docs` issues require clearly defined Acceptance Criteria, as defined above
|
||||
|
||||
## Additional triaging processes and labels
|
||||
|
||||
Before removing the `needs-triage` label, consider adding any of the following labels below.
|
||||
|
||||
| Label | Description |
|
||||
| - | - |
|
||||
| `discuss` | Some issues require discussion with the internal team. Adding this label will automatically open up an internal discussion with the team to facilitate this discussion. |
|
||||
| `core` | Defines what we would like to do internally. We tend to lean towards `help wanted` by default, and adding `core` should be reserved for trickier issues or implementations we have strong opinions/preferences about. |
|
||||
| `good first issue` | Used to denote when an issue may be a good candidate for a first-time contributor to the CLI. These are usually small and well defined issues. |
|
||||
| `help wanted` | Defines what we feel the community could solve should they care to contribute, respectively. We tend to lean towards `help wanted` by default, and adding `core` should be reserved for trickier issues or implementations we have strong opinions/preferences about. |
|
||||
| `invalid` | Added to spam and abusive issues. |
|
||||
| `needs-user-input` | After asking any contributors for more information, add this label so it is clear that the issue has been responded to and we are waiting on the user. |
|
||||
|
||||
## Expectations for community pull requests
|
||||
|
||||
|
|
@ -29,36 +53,13 @@ All incoming pull requests are assigned to one of the engineers for review on a
|
|||
The person in a triage role for a week could take a glance at these pull requests, mostly to see whether
|
||||
the changeset is feasible and to allow the associated CI run for new contributors.
|
||||
|
||||
## Issue triage flowchart
|
||||
## Spam and abuse
|
||||
|
||||
- can this be closed outright?
|
||||
- e.g. spam/junk
|
||||
- add the `invalid` label
|
||||
- close without comment
|
||||
- do we not want to do it?
|
||||
- e.g. we have already discussed not wanting to do or it's a duplicate issue
|
||||
- add the appropriate label (e.g. `duplicate`)
|
||||
- comment and close
|
||||
- do we want external contribution for this?
|
||||
- e.g. the task is relatively straightforward, but the core team does not have the bandwidth to take it on
|
||||
- ensure that the thread contains all the context necessary for someone new to pick this up
|
||||
- add the `help wanted` label
|
||||
- consider adding `good first issue` label
|
||||
- do we want external design contribution for this?
|
||||
- e.g. the task is worthwhile, but needs design work to flesh out the details before implementation and the core team does not have the bandwidth to take it on
|
||||
- ensure that the thread contains all the context necessary for someone new to pick this up
|
||||
- add both the `help wanted` and `needs-design` labels
|
||||
- do we want to do it?
|
||||
- add the `core` label
|
||||
- comment acknowledging that
|
||||
- is it intriguing, but requires discussion?
|
||||
- Add the `discuss` label
|
||||
- Add the `needs-investigation` label if engineering research is required before action can be taken
|
||||
- does it need more info from the issue author?
|
||||
- ask the user for details
|
||||
- add the `needs-user-input` label
|
||||
- is it a usage/support question?
|
||||
- Convert the Issue to a Discussion
|
||||
The primary goal of triaging spam and abuse is to remove distracting and offensive content from our community.
|
||||
|
||||
We get a lot of spam. Whenever you determine an issue as spam, add the `invalid` label and close it as "won't do". For spammy comments, simply mark them as spam using GitHub's built-in spam feature.
|
||||
|
||||
Abusive contributions are defined by our [Code of Conduct](../.github/CODE-OF-CONDUCT.md). Any contribution you determine abusive should be removed. Repeat offenses or particularly offensive abuse should be reported using GitHub's reporting features and the user blocked. If an entire issue is abusive, label it as `invalid` and close as "won't do".
|
||||
|
||||
## Weekly PR audit
|
||||
|
||||
|
|
|
|||
|
|
@ -389,7 +389,6 @@ func (c *Client) ReadBranchConfig(ctx context.Context, branch string) (cfg Branc
|
|||
return
|
||||
}
|
||||
|
||||
cfg.LocalName = branch
|
||||
for _, line := range outputLines(out) {
|
||||
parts := strings.SplitN(line, " ", 2)
|
||||
if len(parts) < 2 {
|
||||
|
|
|
|||
|
|
@ -737,7 +737,7 @@ func TestClientReadBranchConfig(t *testing.T) {
|
|||
name: "read branch config",
|
||||
cmdStdout: "branch.trunk.remote origin\nbranch.trunk.merge refs/heads/trunk\nbranch.trunk.gh-merge-base trunk",
|
||||
wantCmdArgs: `path/to/git config --get-regexp ^branch\.trunk\.(remote|merge|gh-merge-base)$`,
|
||||
wantBranchConfig: BranchConfig{LocalName: "trunk", RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"},
|
||||
wantBranchConfig: BranchConfig{RemoteName: "origin", MergeRef: "refs/heads/trunk", MergeBase: "trunk"},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
|
|
|
|||
|
|
@ -54,16 +54,6 @@ type Ref struct {
|
|||
Name string
|
||||
}
|
||||
|
||||
// TrackingRef represents a ref for a remote tracking branch.
|
||||
type TrackingRef struct {
|
||||
RemoteName string
|
||||
BranchName string
|
||||
}
|
||||
|
||||
func (r TrackingRef) String() string {
|
||||
return "refs/remotes/" + r.RemoteName + "/" + r.BranchName
|
||||
}
|
||||
|
||||
type Commit struct {
|
||||
Sha string
|
||||
Title string
|
||||
|
|
@ -71,8 +61,6 @@ type Commit struct {
|
|||
}
|
||||
|
||||
type BranchConfig struct {
|
||||
// LocalName of the branch.
|
||||
LocalName string
|
||||
RemoteName string
|
||||
RemoteURL *url.URL
|
||||
// MergeBase is the optional base branch to target in a new PR if `--base` is not specified.
|
||||
|
|
|
|||
2
go.mod
2
go.mod
|
|
@ -11,7 +11,7 @@ require (
|
|||
github.com/cenkalti/backoff/v4 v4.3.0
|
||||
github.com/charmbracelet/glamour v0.7.0
|
||||
github.com/charmbracelet/lipgloss v0.10.1-0.20240413172830-d0be07ea6b9c
|
||||
github.com/cli/go-gh/v2 v2.11.1
|
||||
github.com/cli/go-gh/v2 v2.11.2
|
||||
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24
|
||||
github.com/cli/oauth v1.1.1
|
||||
github.com/cli/safeexec v1.0.1
|
||||
|
|
|
|||
4
go.sum
4
go.sum
|
|
@ -95,8 +95,8 @@ github.com/charmbracelet/x/exp/term v0.0.0-20240425164147-ba2a9512b05f/go.mod h1
|
|||
github.com/cli/browser v1.0.0/go.mod h1:IEWkHYbLjkhtjwwWlwTHW2lGxeS5gezEQBMLTwDHf5Q=
|
||||
github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo=
|
||||
github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk=
|
||||
github.com/cli/go-gh/v2 v2.11.1 h1:amAyfqMWQTBdue8iTmDUegGZK7c8kk6WCxD9l/wLtGI=
|
||||
github.com/cli/go-gh/v2 v2.11.1/go.mod h1:MeRoKzXff3ygHu7zP+NVTT+imcHW6p3tpuxHAzRM2xE=
|
||||
github.com/cli/go-gh/v2 v2.11.2 h1:oad1+sESTPNTiTvh3I3t8UmxuovNDxhwLzeMHk45Q9w=
|
||||
github.com/cli/go-gh/v2 v2.11.2/go.mod h1:vVFhi3TfjseIW26ED9itAR8gQK0aVThTm8sYrsZ5QTI=
|
||||
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24 h1:QDrhR4JA2n3ij9YQN0u5ZeuvRIIvsUGmf5yPlTS0w8E=
|
||||
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24/go.mod h1:rr9GNING0onuVw8MnracQHn7PcchnFlP882Y0II2KZk=
|
||||
github.com/cli/oauth v1.1.1 h1:459gD3hSjlKX9B1uXBuiAMdpXBUQ9QGf/NDcCpoQxPs=
|
||||
|
|
|
|||
|
|
@ -70,8 +70,9 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
|
|||
|
||||
Alternatively, use %[1]s--with-token%[1]s to pass in a personal access token (classic) on standard input.
|
||||
The minimum required scopes for the token are: %[1]srepo%[1]s, %[1]sread:org%[1]s, and %[1]sgist%[1]s.
|
||||
|
||||
Fine-grained personal access tokens are not supported.
|
||||
Take care when passing a fine-grained personal access token to %[1]s--with-token%[1]s
|
||||
as the inherent scoping to certain resources may cause confusing behaviour when interacting with other
|
||||
resources. Favour setting %[1]sGH_TOKEN$%[1]s for fine-grained personal access token usage.
|
||||
|
||||
Alternatively, gh will use the authentication token found in environment variables.
|
||||
This method is most suitable for "headless" use of gh such as in automation. See
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ jobs:
|
|||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: cli/gh-extension-precompile@v1
|
||||
- uses: cli/gh-extension-precompile@v2
|
||||
with:
|
||||
generate_attestations: true
|
||||
go_version_file: go.mod
|
||||
|
|
|
|||
|
|
@ -11,6 +11,6 @@ jobs:
|
|||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: cli/gh-extension-precompile@v1
|
||||
- uses: cli/gh-extension-precompile@v2
|
||||
with:
|
||||
build_script_override: "script/build.sh"
|
||||
|
|
|
|||
|
|
@ -518,44 +518,80 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
|
|||
return nil
|
||||
}
|
||||
|
||||
func determineTrackingBranch(gitClient *git.Client, remotes ghContext.Remotes, headBranchConfig *git.BranchConfig) *git.TrackingRef {
|
||||
refsForLookup := []string{"HEAD"}
|
||||
var trackingRefs []git.TrackingRef
|
||||
// trackingRef represents a ref for a remote tracking branch.
|
||||
type trackingRef struct {
|
||||
remoteName string
|
||||
branchName string
|
||||
}
|
||||
|
||||
if headBranchConfig.RemoteName != "" {
|
||||
tr := git.TrackingRef{
|
||||
RemoteName: headBranchConfig.RemoteName,
|
||||
BranchName: strings.TrimPrefix(headBranchConfig.MergeRef, "refs/heads/"),
|
||||
func (r trackingRef) String() string {
|
||||
return "refs/remotes/" + r.remoteName + "/" + r.branchName
|
||||
}
|
||||
|
||||
func mustParseTrackingRef(text string) trackingRef {
|
||||
parts := strings.SplitN(string(text), "/", 4)
|
||||
// The only place this is called is tryDetermineTrackingRef, where we are reconstructing
|
||||
// the same tracking ref we passed in. If it doesn't match the expected format, this is a
|
||||
// programmer error we want to know about, so it's ok to panic.
|
||||
if len(parts) != 4 {
|
||||
panic(fmt.Errorf("tracking ref should have four parts: %s", text))
|
||||
}
|
||||
if parts[0] != "refs" || parts[1] != "remotes" {
|
||||
panic(fmt.Errorf("tracking ref should start with refs/remotes/: %s", text))
|
||||
}
|
||||
|
||||
return trackingRef{
|
||||
remoteName: parts[2],
|
||||
branchName: parts[3],
|
||||
}
|
||||
}
|
||||
|
||||
// tryDetermineTrackingRef is intended to try and find a remote branch on the same commit as the currently checked out
|
||||
// HEAD, i.e. the local branch. If there are multiple branches that might match, the first remote is chosen, which in
|
||||
// practice is determined by the sorting algorithm applied much earlier in the process, roughly "upstream", "github", "origin",
|
||||
// and then everything else unstably sorted.
|
||||
func tryDetermineTrackingRef(gitClient *git.Client, remotes ghContext.Remotes, localBranchName string, headBranchConfig git.BranchConfig) (trackingRef, bool) {
|
||||
// To try and determine the tracking ref for a local branch, we first construct a collection of refs
|
||||
// that might be tracking, given the current branch's config, and the list of known remotes.
|
||||
refsForLookup := []string{"HEAD"}
|
||||
if headBranchConfig.RemoteName != "" && headBranchConfig.MergeRef != "" {
|
||||
tr := trackingRef{
|
||||
remoteName: headBranchConfig.RemoteName,
|
||||
branchName: strings.TrimPrefix(headBranchConfig.MergeRef, "refs/heads/"),
|
||||
}
|
||||
trackingRefs = append(trackingRefs, tr)
|
||||
refsForLookup = append(refsForLookup, tr.String())
|
||||
}
|
||||
|
||||
for _, remote := range remotes {
|
||||
tr := git.TrackingRef{
|
||||
RemoteName: remote.Name,
|
||||
BranchName: headBranchConfig.LocalName,
|
||||
tr := trackingRef{
|
||||
remoteName: remote.Name,
|
||||
branchName: localBranchName,
|
||||
}
|
||||
trackingRefs = append(trackingRefs, tr)
|
||||
refsForLookup = append(refsForLookup, tr.String())
|
||||
}
|
||||
|
||||
// Then we ask git for details about these refs, for example, refs/remotes/origin/trunk might return a hash
|
||||
// for the remote tracking branch, trunk, for the remote, origin. If there is no ref, the git client returns
|
||||
// no ref information.
|
||||
//
|
||||
// We also first check for the HEAD ref, so that we have the hash of the currently checked out commit.
|
||||
resolvedRefs, _ := gitClient.ShowRefs(context.Background(), refsForLookup)
|
||||
|
||||
// If there is more than one resolved ref, that means that at least one ref was found in addition to the HEAD.
|
||||
if len(resolvedRefs) > 1 {
|
||||
headRef := resolvedRefs[0]
|
||||
for _, r := range resolvedRefs[1:] {
|
||||
if r.Hash != resolvedRefs[0].Hash {
|
||||
// If the hash of the remote ref doesn't match the hash of HEAD then the remote branch is not in the same
|
||||
// state, so it can't be used.
|
||||
if r.Hash != headRef.Hash {
|
||||
continue
|
||||
}
|
||||
for _, tr := range trackingRefs {
|
||||
if tr.String() != r.Name {
|
||||
continue
|
||||
}
|
||||
return &tr
|
||||
}
|
||||
// Otherwise we can parse the returned ref into a tracking ref and return that
|
||||
return mustParseTrackingRef(r.Name), true
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
return trackingRef{}, false
|
||||
}
|
||||
|
||||
func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadataState, error) {
|
||||
|
|
@ -647,14 +683,14 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
|
|||
headBranchConfig := gitClient.ReadBranchConfig(context.Background(), headBranch)
|
||||
if isPushEnabled {
|
||||
// determine whether the head branch is already pushed to a remote
|
||||
if pushedTo := determineTrackingBranch(gitClient, remotes, &headBranchConfig); pushedTo != nil {
|
||||
if trackingRef, found := tryDetermineTrackingRef(gitClient, remotes, headBranch, headBranchConfig); found {
|
||||
isPushEnabled = false
|
||||
if r, err := remotes.FindByName(pushedTo.RemoteName); err == nil {
|
||||
if r, err := remotes.FindByName(trackingRef.remoteName); err == nil {
|
||||
headRepo = r
|
||||
headRemote = r
|
||||
headBranchLabel = pushedTo.BranchName
|
||||
headBranchLabel = trackingRef.branchName
|
||||
if !ghrepo.IsSame(baseRepo, headRepo) {
|
||||
headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), pushedTo.BranchName)
|
||||
headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), trackingRef.branchName)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1622,12 +1622,13 @@ func Test_createRun(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func Test_determineTrackingBranch(t *testing.T) {
|
||||
func Test_tryDetermineTrackingRef(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
cmdStubs func(*run.CommandStubber)
|
||||
remotes context.Remotes
|
||||
assert func(ref *git.TrackingRef, t *testing.T)
|
||||
name string
|
||||
cmdStubs func(*run.CommandStubber)
|
||||
remotes context.Remotes
|
||||
expectedTrackingRef trackingRef
|
||||
expectedFound bool
|
||||
}{
|
||||
{
|
||||
name: "empty",
|
||||
|
|
@ -1635,54 +1636,53 @@ func Test_determineTrackingBranch(t *testing.T) {
|
|||
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
|
||||
cs.Register(`git show-ref --verify -- HEAD`, 0, "abc HEAD")
|
||||
},
|
||||
assert: func(ref *git.TrackingRef, t *testing.T) {
|
||||
assert.Nil(t, ref)
|
||||
},
|
||||
expectedTrackingRef: trackingRef{},
|
||||
expectedFound: false,
|
||||
},
|
||||
{
|
||||
name: "no match",
|
||||
cmdStubs: func(cs *run.CommandStubber) {
|
||||
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
|
||||
cs.Register("git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature", 0, "abc HEAD\nbca refs/remotes/origin/feature")
|
||||
cs.Register("git show-ref --verify -- HEAD refs/remotes/upstream/feature refs/remotes/origin/feature", 0, "abc HEAD\nbca refs/remotes/upstream/feature")
|
||||
},
|
||||
remotes: context.Remotes{
|
||||
&context.Remote{
|
||||
Remote: &git.Remote{Name: "origin"},
|
||||
Repo: ghrepo.New("hubot", "Spoon-Knife"),
|
||||
},
|
||||
&context.Remote{
|
||||
Remote: &git.Remote{Name: "upstream"},
|
||||
Repo: ghrepo.New("octocat", "Spoon-Knife"),
|
||||
},
|
||||
&context.Remote{
|
||||
Remote: &git.Remote{Name: "origin"},
|
||||
Repo: ghrepo.New("hubot", "Spoon-Knife"),
|
||||
},
|
||||
},
|
||||
assert: func(ref *git.TrackingRef, t *testing.T) {
|
||||
assert.Nil(t, ref)
|
||||
},
|
||||
expectedTrackingRef: trackingRef{},
|
||||
expectedFound: false,
|
||||
},
|
||||
{
|
||||
name: "match",
|
||||
cmdStubs: func(cs *run.CommandStubber) {
|
||||
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
|
||||
cs.Register(`git show-ref --verify -- HEAD refs/remotes/origin/feature refs/remotes/upstream/feature$`, 0, heredoc.Doc(`
|
||||
cs.Register(`git show-ref --verify -- HEAD refs/remotes/upstream/feature refs/remotes/origin/feature$`, 0, heredoc.Doc(`
|
||||
deadbeef HEAD
|
||||
deadb00f refs/remotes/origin/feature
|
||||
deadbeef refs/remotes/upstream/feature
|
||||
deadb00f refs/remotes/upstream/feature
|
||||
deadbeef refs/remotes/origin/feature
|
||||
`))
|
||||
},
|
||||
remotes: context.Remotes{
|
||||
&context.Remote{
|
||||
Remote: &git.Remote{Name: "origin"},
|
||||
Repo: ghrepo.New("hubot", "Spoon-Knife"),
|
||||
},
|
||||
&context.Remote{
|
||||
Remote: &git.Remote{Name: "upstream"},
|
||||
Repo: ghrepo.New("octocat", "Spoon-Knife"),
|
||||
},
|
||||
&context.Remote{
|
||||
Remote: &git.Remote{Name: "origin"},
|
||||
Repo: ghrepo.New("hubot", "Spoon-Knife"),
|
||||
},
|
||||
},
|
||||
assert: func(ref *git.TrackingRef, t *testing.T) {
|
||||
assert.Equal(t, "upstream", ref.RemoteName)
|
||||
assert.Equal(t, "feature", ref.BranchName)
|
||||
expectedTrackingRef: trackingRef{
|
||||
remoteName: "origin",
|
||||
branchName: "feature",
|
||||
},
|
||||
expectedFound: true,
|
||||
},
|
||||
{
|
||||
name: "respect tracking config",
|
||||
|
|
@ -1702,9 +1702,8 @@ func Test_determineTrackingBranch(t *testing.T) {
|
|||
Repo: ghrepo.New("hubot", "Spoon-Knife"),
|
||||
},
|
||||
},
|
||||
assert: func(ref *git.TrackingRef, t *testing.T) {
|
||||
assert.Nil(t, ref)
|
||||
},
|
||||
expectedTrackingRef: trackingRef{},
|
||||
expectedFound: false,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
|
|
@ -1719,8 +1718,10 @@ func Test_determineTrackingBranch(t *testing.T) {
|
|||
GitPath: "some/path/git",
|
||||
}
|
||||
headBranchConfig := gitClient.ReadBranchConfig(ctx.Background(), "feature")
|
||||
ref := determineTrackingBranch(gitClient, tt.remotes, &headBranchConfig)
|
||||
tt.assert(ref, t)
|
||||
ref, found := tryDetermineTrackingRef(gitClient, tt.remotes, "feature", headBranchConfig)
|
||||
|
||||
assert.Equal(t, tt.expectedTrackingRef, ref)
|
||||
assert.Equal(t, tt.expectedFound, found)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -66,22 +66,27 @@ type EditOptions struct {
|
|||
}
|
||||
|
||||
type EditRepositoryInput struct {
|
||||
AllowForking *bool `json:"allow_forking,omitempty"`
|
||||
AllowUpdateBranch *bool `json:"allow_update_branch,omitempty"`
|
||||
DefaultBranch *string `json:"default_branch,omitempty"`
|
||||
DeleteBranchOnMerge *bool `json:"delete_branch_on_merge,omitempty"`
|
||||
Description *string `json:"description,omitempty"`
|
||||
EnableAutoMerge *bool `json:"allow_auto_merge,omitempty"`
|
||||
EnableIssues *bool `json:"has_issues,omitempty"`
|
||||
EnableMergeCommit *bool `json:"allow_merge_commit,omitempty"`
|
||||
EnableProjects *bool `json:"has_projects,omitempty"`
|
||||
EnableDiscussions *bool `json:"has_discussions,omitempty"`
|
||||
EnableRebaseMerge *bool `json:"allow_rebase_merge,omitempty"`
|
||||
EnableSquashMerge *bool `json:"allow_squash_merge,omitempty"`
|
||||
EnableWiki *bool `json:"has_wiki,omitempty"`
|
||||
Homepage *string `json:"homepage,omitempty"`
|
||||
IsTemplate *bool `json:"is_template,omitempty"`
|
||||
Visibility *string `json:"visibility,omitempty"`
|
||||
enableAdvancedSecurity *bool
|
||||
enableSecretScanning *bool
|
||||
enableSecretScanningPushProtection *bool
|
||||
|
||||
AllowForking *bool `json:"allow_forking,omitempty"`
|
||||
AllowUpdateBranch *bool `json:"allow_update_branch,omitempty"`
|
||||
DefaultBranch *string `json:"default_branch,omitempty"`
|
||||
DeleteBranchOnMerge *bool `json:"delete_branch_on_merge,omitempty"`
|
||||
Description *string `json:"description,omitempty"`
|
||||
EnableAutoMerge *bool `json:"allow_auto_merge,omitempty"`
|
||||
EnableIssues *bool `json:"has_issues,omitempty"`
|
||||
EnableMergeCommit *bool `json:"allow_merge_commit,omitempty"`
|
||||
EnableProjects *bool `json:"has_projects,omitempty"`
|
||||
EnableDiscussions *bool `json:"has_discussions,omitempty"`
|
||||
EnableRebaseMerge *bool `json:"allow_rebase_merge,omitempty"`
|
||||
EnableSquashMerge *bool `json:"allow_squash_merge,omitempty"`
|
||||
EnableWiki *bool `json:"has_wiki,omitempty"`
|
||||
Homepage *string `json:"homepage,omitempty"`
|
||||
IsTemplate *bool `json:"is_template,omitempty"`
|
||||
SecurityAndAnalysis *SecurityAndAnalysisInput `json:"security_and_analysis,omitempty"`
|
||||
Visibility *string `json:"visibility,omitempty"`
|
||||
}
|
||||
|
||||
func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobra.Command {
|
||||
|
|
@ -157,6 +162,10 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr
|
|||
return cmdutil.FlagErrorf("use of --visibility flag requires --accept-visibility-change-consequences flag")
|
||||
}
|
||||
|
||||
if hasSecurityEdits(opts.Edits) {
|
||||
opts.Edits.SecurityAndAnalysis = transformSecurityAndAnalysisOpts(opts)
|
||||
}
|
||||
|
||||
if runF != nil {
|
||||
return runF(opts)
|
||||
}
|
||||
|
|
@ -177,6 +186,9 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr
|
|||
cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableSquashMerge, "enable-squash-merge", "", "Enable merging pull requests via squashed commit")
|
||||
cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableRebaseMerge, "enable-rebase-merge", "", "Enable merging pull requests via rebase")
|
||||
cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableAutoMerge, "enable-auto-merge", "", "Enable auto-merge functionality")
|
||||
cmdutil.NilBoolFlag(cmd, &opts.Edits.enableAdvancedSecurity, "enable-advanced-security", "", "Enable advanced security in the repository")
|
||||
cmdutil.NilBoolFlag(cmd, &opts.Edits.enableSecretScanning, "enable-secret-scanning", "", "Enable secret scanning in the repository")
|
||||
cmdutil.NilBoolFlag(cmd, &opts.Edits.enableSecretScanningPushProtection, "enable-secret-scanning-push-protection", "", "Enable secret scanning push protection in the repository. Secret scanning must be enabled first")
|
||||
cmdutil.NilBoolFlag(cmd, &opts.Edits.DeleteBranchOnMerge, "delete-branch-on-merge", "", "Delete head branch when pull requests are merged")
|
||||
cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowForking, "allow-forking", "", "Allow forking of an organization repository")
|
||||
cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowUpdateBranch, "allow-update-branch", "", "Allow a pull request head branch that is behind its base branch to be updated")
|
||||
|
|
@ -240,6 +252,17 @@ func editRun(ctx context.Context, opts *EditOptions) error {
|
|||
}
|
||||
}
|
||||
|
||||
if opts.Edits.SecurityAndAnalysis != nil {
|
||||
apiClient := api.NewClientFromHTTP(opts.HTTPClient)
|
||||
repo, err := api.FetchRepository(apiClient, opts.Repository, []string{"viewerCanAdminister"})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if !repo.ViewerCanAdminister {
|
||||
return fmt.Errorf("you do not have sufficient permissions to edit repository security and analysis features")
|
||||
}
|
||||
}
|
||||
|
||||
apiPath := fmt.Sprintf("repos/%s/%s", repo.RepoOwner(), repo.RepoName())
|
||||
|
||||
body := &bytes.Buffer{}
|
||||
|
|
@ -560,3 +583,49 @@ func isIncluded(value string, opts []string) bool {
|
|||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func boolToStatus(status bool) *string {
|
||||
var result string
|
||||
if status {
|
||||
result = "enabled"
|
||||
} else {
|
||||
result = "disabled"
|
||||
}
|
||||
return &result
|
||||
}
|
||||
|
||||
func hasSecurityEdits(edits EditRepositoryInput) bool {
|
||||
return edits.enableAdvancedSecurity != nil || edits.enableSecretScanning != nil || edits.enableSecretScanningPushProtection != nil
|
||||
}
|
||||
|
||||
type SecurityAndAnalysisInput struct {
|
||||
EnableAdvancedSecurity *SecurityAndAnalysisStatus `json:"advanced_security,omitempty"`
|
||||
EnableSecretScanning *SecurityAndAnalysisStatus `json:"secret_scanning,omitempty"`
|
||||
EnableSecretScanningPushProtection *SecurityAndAnalysisStatus `json:"secret_scanning_push_protection,omitempty"`
|
||||
}
|
||||
|
||||
type SecurityAndAnalysisStatus struct {
|
||||
Status *string `json:"status,omitempty"`
|
||||
}
|
||||
|
||||
// Transform security and analysis parameters to properly serialize EditRepositoryInput
|
||||
// See API Docs: https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#update-a-repository
|
||||
func transformSecurityAndAnalysisOpts(opts *EditOptions) *SecurityAndAnalysisInput {
|
||||
securityOptions := &SecurityAndAnalysisInput{}
|
||||
if opts.Edits.enableAdvancedSecurity != nil {
|
||||
securityOptions.EnableAdvancedSecurity = &SecurityAndAnalysisStatus{
|
||||
Status: boolToStatus(*opts.Edits.enableAdvancedSecurity),
|
||||
}
|
||||
}
|
||||
if opts.Edits.enableSecretScanning != nil {
|
||||
securityOptions.EnableSecretScanning = &SecurityAndAnalysisStatus{
|
||||
Status: boolToStatus(*opts.Edits.enableSecretScanning),
|
||||
}
|
||||
}
|
||||
if opts.Edits.enableSecretScanningPushProtection != nil {
|
||||
securityOptions.EnableSecretScanningPushProtection = &SecurityAndAnalysisStatus{
|
||||
Status: boolToStatus(*opts.Edits.enableSecretScanningPushProtection),
|
||||
}
|
||||
}
|
||||
return securityOptions
|
||||
}
|
||||
|
|
|
|||
|
|
@ -201,6 +201,65 @@ func Test_editRun(t *testing.T) {
|
|||
}))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "enable/disable security and analysis settings",
|
||||
opts: EditOptions{
|
||||
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
|
||||
Edits: EditRepositoryInput{
|
||||
SecurityAndAnalysis: &SecurityAndAnalysisInput{
|
||||
EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
|
||||
Status: sp("enabled"),
|
||||
},
|
||||
EnableSecretScanning: &SecurityAndAnalysisStatus{
|
||||
Status: sp("enabled"),
|
||||
},
|
||||
EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{
|
||||
Status: sp("disabled"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
httpStubs: func(t *testing.T, r *httpmock.Registry) {
|
||||
r.Register(
|
||||
httpmock.GraphQL(`query RepositoryInfo\b`),
|
||||
httpmock.StringResponse(`{"data": { "repository": { "viewerCanAdminister": true } } }`))
|
||||
|
||||
r.Register(
|
||||
httpmock.REST("PATCH", "repos/OWNER/REPO"),
|
||||
httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) {
|
||||
assert.Equal(t, 1, len(payload))
|
||||
securityAndAnalysis := payload["security_and_analysis"].(map[string]interface{})
|
||||
assert.Equal(t, "enabled", securityAndAnalysis["advanced_security"].(map[string]interface{})["status"])
|
||||
assert.Equal(t, "enabled", securityAndAnalysis["secret_scanning"].(map[string]interface{})["status"])
|
||||
assert.Equal(t, "disabled", securityAndAnalysis["secret_scanning_push_protection"].(map[string]interface{})["status"])
|
||||
}))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "does not have sufficient permissions for security edits",
|
||||
opts: EditOptions{
|
||||
Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"),
|
||||
Edits: EditRepositoryInput{
|
||||
SecurityAndAnalysis: &SecurityAndAnalysisInput{
|
||||
EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
|
||||
Status: sp("enabled"),
|
||||
},
|
||||
EnableSecretScanning: &SecurityAndAnalysisStatus{
|
||||
Status: sp("enabled"),
|
||||
},
|
||||
EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{
|
||||
Status: sp("disabled"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
httpStubs: func(t *testing.T, r *httpmock.Registry) {
|
||||
r.Register(
|
||||
httpmock.GraphQL(`query RepositoryInfo\b`),
|
||||
httpmock.StringResponse(`{"data": { "repository": { "viewerCanAdminister": false } } }`))
|
||||
},
|
||||
wantsErr: "you do not have sufficient permissions to edit repository security and analysis features",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
@ -670,6 +729,95 @@ func Test_editRun_interactive(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func Test_transformSecurityAndAnalysisOpts(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
opts EditOptions
|
||||
want *SecurityAndAnalysisInput
|
||||
}{
|
||||
{
|
||||
name: "Enable all security and analysis settings",
|
||||
opts: EditOptions{
|
||||
Edits: EditRepositoryInput{
|
||||
enableAdvancedSecurity: bp(true),
|
||||
enableSecretScanning: bp(true),
|
||||
enableSecretScanningPushProtection: bp(true),
|
||||
},
|
||||
},
|
||||
want: &SecurityAndAnalysisInput{
|
||||
EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
|
||||
Status: sp("enabled"),
|
||||
},
|
||||
EnableSecretScanning: &SecurityAndAnalysisStatus{
|
||||
Status: sp("enabled"),
|
||||
},
|
||||
EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{
|
||||
Status: sp("enabled"),
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Disable all security and analysis settings",
|
||||
opts: EditOptions{
|
||||
Edits: EditRepositoryInput{
|
||||
enableAdvancedSecurity: bp(false),
|
||||
enableSecretScanning: bp(false),
|
||||
enableSecretScanningPushProtection: bp(false),
|
||||
},
|
||||
},
|
||||
want: &SecurityAndAnalysisInput{
|
||||
EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
|
||||
Status: sp("disabled"),
|
||||
},
|
||||
EnableSecretScanning: &SecurityAndAnalysisStatus{
|
||||
Status: sp("disabled"),
|
||||
},
|
||||
EnableSecretScanningPushProtection: &SecurityAndAnalysisStatus{
|
||||
Status: sp("disabled"),
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Enable only advanced security",
|
||||
opts: EditOptions{
|
||||
Edits: EditRepositoryInput{
|
||||
enableAdvancedSecurity: bp(true),
|
||||
},
|
||||
},
|
||||
want: &SecurityAndAnalysisInput{
|
||||
EnableAdvancedSecurity: &SecurityAndAnalysisStatus{
|
||||
Status: sp("enabled"),
|
||||
},
|
||||
EnableSecretScanning: nil,
|
||||
EnableSecretScanningPushProtection: nil,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "Disable only secret scanning",
|
||||
opts: EditOptions{
|
||||
Edits: EditRepositoryInput{
|
||||
enableSecretScanning: bp(false),
|
||||
},
|
||||
},
|
||||
want: &SecurityAndAnalysisInput{
|
||||
EnableAdvancedSecurity: nil,
|
||||
EnableSecretScanning: &SecurityAndAnalysisStatus{
|
||||
Status: sp("disabled"),
|
||||
},
|
||||
EnableSecretScanningPushProtection: nil,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
opts := &tt.opts
|
||||
transformed := transformSecurityAndAnalysisOpts(opts)
|
||||
assert.Equal(t, tt.want, transformed)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func sp(v string) *string {
|
||||
return &v
|
||||
}
|
||||
|
|
|
|||
|
|
@ -221,6 +221,8 @@ func forkRun(opts *ForkOptions) error {
|
|||
} else {
|
||||
if connectedToTerminal {
|
||||
fmt.Fprintf(stderr, "%s Created fork %s\n", cs.SuccessIconWithColor(cs.Green), cs.Bold(ghrepo.FullName(forkedRepo)))
|
||||
} else {
|
||||
fmt.Fprintln(opts.IO.Out, ghrepo.GenerateRepoURL(forkedRepo, ""))
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -390,6 +390,7 @@ func TestRepoFork(t *testing.T) {
|
|||
},
|
||||
},
|
||||
httpStubs: forkPost,
|
||||
wantOut: "https://github.com/someone/REPO\n",
|
||||
},
|
||||
{
|
||||
name: "implicit nontty remote exists",
|
||||
|
|
@ -424,11 +425,13 @@ func TestRepoFork(t *testing.T) {
|
|||
cs.Register("git remote rename origin upstream", 0, "")
|
||||
cs.Register(`git remote add origin https://github.com/someone/REPO.git`, 0, "")
|
||||
},
|
||||
wantOut: "https://github.com/someone/REPO\n",
|
||||
},
|
||||
{
|
||||
name: "implicit nontty no args",
|
||||
opts: &ForkOptions{},
|
||||
httpStubs: forkPost,
|
||||
wantOut: "https://github.com/someone/REPO\n",
|
||||
},
|
||||
{
|
||||
name: "passes git flags",
|
||||
|
|
@ -561,6 +564,7 @@ func TestRepoFork(t *testing.T) {
|
|||
Repository: "OWNER/REPO",
|
||||
},
|
||||
httpStubs: forkPost,
|
||||
wantOut: "https://github.com/someone/REPO\n",
|
||||
},
|
||||
{
|
||||
name: "repo arg nontty repo already exists",
|
||||
|
|
@ -604,6 +608,7 @@ func TestRepoFork(t *testing.T) {
|
|||
cs.Register(`git -C REPO fetch upstream`, 0, "")
|
||||
cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "")
|
||||
},
|
||||
wantOut: "https://github.com/someone/REPO\n",
|
||||
},
|
||||
{
|
||||
name: "non tty repo arg with fork-name",
|
||||
|
|
@ -640,6 +645,7 @@ func TestRepoFork(t *testing.T) {
|
|||
httpmock.StringResponse(renameResult))
|
||||
},
|
||||
wantErrOut: "",
|
||||
wantOut: "https://github.com/OWNER/REPO\n",
|
||||
},
|
||||
{
|
||||
name: "tty repo arg with fork-name",
|
||||
|
|
@ -694,6 +700,7 @@ func TestRepoFork(t *testing.T) {
|
|||
cs.Register(`git -C REPO fetch upstream`, 0, "")
|
||||
cs.Register(`git -C REPO config --add remote.upstream.gh-resolved base`, 0, "")
|
||||
},
|
||||
wantOut: "https://github.com/someone/REPO\n",
|
||||
},
|
||||
{
|
||||
name: "does not retry clone if error occurs and exit code is not 128",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue