ronantakizawa/codereview-bench huggingface dataset:
Repository: cli/cli
File: pkg/cmd/extension/manager.go (Go)

Code before:
}

// Install installs an extension from repo, and pins to commitish if provided
func (m *Manager) Install(repo ghrepo.Interface, target string) error {
	isBin, err := isBinExtension(m.client, repo)
	if err != nil {
		if errors.Is(err, releaseNotFoundErr) {
			if ok, err := repoExists(m.client, repo); err != nil {
				return err
			} else if !ok {
				return repositoryNotFoundErr
			}
		} else {
			return fmt.Errorf("could not check for binary extension: %w", err)
		}
	}
	if isBin {
		return m.installBin(repo, target)
	}

	hs, err := hasScript(m.client, repo)
	if err != nil {
		return err
	}
	if !hs {
		return fmt.Errorf("extension is not installable: no acceptable release artifact or script found in %s", repo)
	}

	return m.installGit(repo, target)
}

func (m *Manager) installBin(repo ghrepo.Interface, target string) error {
	var r *release
	var err error
	isPinned := target != ""
	if isPinned {
		r, err = fetchReleaseFromTag(m.client, repo, target)
	} else {
		r, err = fetchLatestRelease(m.client, repo)
	}
	if err != nil {
		return err
	}

	platform, ext := m.platform()
	isMacARM := platform == "darwin-arm64"
	trueARMBinary := false

	var asset *releaseAsset
	for _, a := range r.Assets {
		if strings.HasSuffix(a.Name, platform+ext) {

Review comment (suggestion):
```suggestion
		return fmt.Errorf("extension is not installable: no usable release artifact or script found in %s", repo)
```

Code after:
}

// Install installs an extension from repo, and pins to commitish if provided
func (m *Manager) Install(repo ghrepo.Interface, target string) error {
	isBin, err := isBinExtension(m.client, repo)
	if err != nil {
		if errors.Is(err, releaseNotFoundErr) {
			if ok, err := repoExists(m.client, repo); err != nil {
				return err
			} else if !ok {
				return repositoryNotFoundErr
			}
		} else {
			return fmt.Errorf("could not check for binary extension: %w", err)
		}
	}
	if isBin {
		return m.installBin(repo, target)
	}

	hs, err := hasScript(m.client, repo)
	if err != nil {
		return err
	}
	if !hs {
		return fmt.Errorf("extension is not installable: no usable release artifact or script found in %s", repo)
	}

	return m.installGit(repo, target)
}

func (m *Manager) installBin(repo ghrepo.Interface, target string) error {
	var r *release
	var err error
	isPinned := target != ""
	if isPinned {
		r, err = fetchReleaseFromTag(m.client, repo, target)
	} else {
		r, err = fetchLatestRelease(m.client, repo)
	}
	if err != nil {
		return err
	}

	platform, ext := m.platform()
	isMacARM := platform == "darwin-arm64"
	trueARMBinary := false

	var asset *releaseAsset
	for _, a := range r.Assets {
		if strings.HasSuffix(a.Name, platform+ext) {
Repository: cli/cli
File: pkg/cmd/attestation/verification/attestation.go (Go)

Code before:
var ErrEmptyBundleFile = errors.New("provided bundle file is empty")

type FetchRemoteAttestationsParams struct {
	Digest string
	Limit  int
	Owner  string
	Repo   string
}

// GetLocalAttestations returns a slice of attestations read from a local bundle file.
func GetLocalAttestations(path string) ([]*api.Attestation, error) {
	var attestations []*api.Attestation
	var err error
	fileExt := filepath.Ext(path)
	if fileExt == ".json" {
		attestations, err = loadBundleFromJSONFile(path)
	} else if fileExt == ".jsonl" {
		attestations, err = loadBundlesFromJSONLinesFile(path)
	} else {
		return nil, ErrUnrecognisedBundleExtension
	}

	if err != nil {
		var pathErr *os.PathError
		if errors.As(err, &pathErr) {
			return nil, fmt.Errorf("could not load content from file path %s", path)
		} else if errors.Is(err, bundle.ErrValidation) {
			return nil, err
		}
		return nil, fmt.Errorf("bundle content could not be parsed")
	}

	return attestations, nil
}

func loadBundleFromJSONFile(path string) ([]*api.Attestation, error) {
	b, err := bundle.LoadJSONFromPath(path)
	if err != nil {
		return nil, err
	}

	return []*api.Attestation{{Bundle: b}}, nil
}

func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) {
	fileContent, err := os.ReadFile(path)
	if err != nil {
		return nil, err
	}

	attestations := []*api.Attestation{}

Review comment (suggestion):
```suggestion
			return nil, fmt.Errorf("could not load content from file path %s: %w", path, err)
```

Don't loos the original error contains valuable information.

Code after:
var ErrEmptyBundleFile = errors.New("provided bundle file is empty")

type FetchRemoteAttestationsParams struct {
	Digest string
	Limit  int
	Owner  string
	Repo   string
}

// GetLocalAttestations returns a slice of attestations read from a local bundle file.
func GetLocalAttestations(path string) ([]*api.Attestation, error) {
	var attestations []*api.Attestation
	var err error
	fileExt := filepath.Ext(path)
	if fileExt == ".json" {
		attestations, err = loadBundleFromJSONFile(path)
	} else if fileExt == ".jsonl" {
		attestations, err = loadBundlesFromJSONLinesFile(path)
	} else {
		return nil, ErrUnrecognisedBundleExtension
	}

	if err != nil {
		var pathErr *os.PathError
		if errors.As(err, &pathErr) {
			return nil, fmt.Errorf("could not load content from file path %s: %w", path, err)
		} else if errors.Is(err, bundle.ErrValidation) {
			return nil, err
		}
		return nil, fmt.Errorf("bundle content could not be parsed: %w", err)
	}

	return attestations, nil
}

func loadBundleFromJSONFile(path string) ([]*api.Attestation, error) {
	b, err := bundle.LoadJSONFromPath(path)
	if err != nil {
		return nil, err
	}

	return []*api.Attestation{{Bundle: b}}, nil
}

func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) {
	fileContent, err := os.ReadFile(path)
	if err != nil {
		return nil, err
	}

	attestations := []*api.Attestation{}
Repository: cli/cli
File: pkg/cmd/attestation/verification/attestation.go (Go)

Code before:
	Limit  int
	Owner  string
	Repo   string
}

// GetLocalAttestations returns a slice of attestations read from a local bundle file.
func GetLocalAttestations(path string) ([]*api.Attestation, error) {
	var attestations []*api.Attestation
	var err error
	fileExt := filepath.Ext(path)
	if fileExt == ".json" {
		attestations, err = loadBundleFromJSONFile(path)
	} else if fileExt == ".jsonl" {
		attestations, err = loadBundlesFromJSONLinesFile(path)
	} else {
		return nil, ErrUnrecognisedBundleExtension
	}

	if err != nil {
		var pathErr *os.PathError
		if errors.As(err, &pathErr) {
			return nil, fmt.Errorf("could not load content from file path %s", path)
		} else if errors.Is(err, bundle.ErrValidation) {
			return nil, err
		}
		return nil, fmt.Errorf("bundle content could not be parsed")
	}

	return attestations, nil
}

func loadBundleFromJSONFile(path string) ([]*api.Attestation, error) {
	b, err := bundle.LoadJSONFromPath(path)
	if err != nil {
		return nil, err
	}

	return []*api.Attestation{{Bundle: b}}, nil
}

func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) {
	fileContent, err := os.ReadFile(path)
	if err != nil {
		return nil, err
	}

	attestations := []*api.Attestation{}

	decoder := json.NewDecoder(bytes.NewReader(fileContent))

	for decoder.More() {

Review comment (suggestion):
```suggestion
		return nil, fmt.Errorf("bundle content could not be parsed: %w", err)
```

Code after:
	Limit  int
	Owner  string
	Repo   string
}

// GetLocalAttestations returns a slice of attestations read from a local bundle file.
func GetLocalAttestations(path string) ([]*api.Attestation, error) {
	var attestations []*api.Attestation
	var err error
	fileExt := filepath.Ext(path)
	if fileExt == ".json" {
		attestations, err = loadBundleFromJSONFile(path)
	} else if fileExt == ".jsonl" {
		attestations, err = loadBundlesFromJSONLinesFile(path)
	} else {
		return nil, ErrUnrecognisedBundleExtension
	}

	if err != nil {
		var pathErr *os.PathError
		if errors.As(err, &pathErr) {
			return nil, fmt.Errorf("could not load content from file path %s: %w", path, err)
		} else if errors.Is(err, bundle.ErrValidation) {
			return nil, err
		}
		return nil, fmt.Errorf("bundle content could not be parsed: %w", err)
	}

	return attestations, nil
}

func loadBundleFromJSONFile(path string) ([]*api.Attestation, error) {
	b, err := bundle.LoadJSONFromPath(path)
	if err != nil {
		return nil, err
	}

	return []*api.Attestation{{Bundle: b}}, nil
}

func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) {
	fileContent, err := os.ReadFile(path)
	if err != nil {
		return nil, err
	}

	attestations := []*api.Attestation{}

	decoder := json.NewDecoder(bytes.NewReader(fileContent))

	for decoder.More() {
Repository: cli/cli
File: pkg/cmd/pr/status/status.go (Go)

Code before:

	HasRepoOverride bool
	Exporter        cmdutil.Exporter
	ConflictStatus  bool

	Detector fd.Detector
}

func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Command {
	opts := &StatusOptions{
		IO:         f.IOStreams,
		HttpClient: f.HttpClient,
		GitClient:  f.GitClient,
		Config:     f.Config,
		Remotes:    f.Remotes,
		Branch:     f.Branch,
	}

	cmd := &cobra.Command{
		Use:   "status",
		Short: "Show status of relevant pull requests",
		Long: heredoc.Docf(`
			Show status of relevant pull requests.

			The status shows a summary of PRs that includes information such as
			PR number, title, CI checks, reviews, etc.

			To see more details of CI checks, run %[1]sgh pr checks%[1]s.
		`, "`"),
		Args: cmdutil.NoArgsQuoteReminder,
		RunE: func(cmd *cobra.Command, args []string) error {
			// support `-R, --repo` override
			opts.BaseRepo = f.BaseRepo
			opts.HasRepoOverride = cmd.Flags().Changed("repo")

			if runF != nil {
				return runF(opts)
			}
			return statusRun(opts)
		},
	}

	cmd.Flags().BoolVarP(&opts.ConflictStatus, "conflict-status", "c", false, "Display the merge conflict status of each pull request")
	cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.PullRequestFields)

	return cmd
}

func statusRun(opts *StatusOptions) error {
	ctx := context.Background()
	httpClient, err := opts.HttpClient()

Review comment (suggestion):
Nit: prefer to use the written form of "pull request" instead of "PR" in help docs.

```suggestion
			The status shows a summary of pull requests that includes information such as
			pull request number, title, CI checks, reviews, etc.
```

Code after:

	HasRepoOverride bool
	Exporter        cmdutil.Exporter
	ConflictStatus  bool

	Detector fd.Detector
}

func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Command {
	opts := &StatusOptions{
		IO:         f.IOStreams,
		HttpClient: f.HttpClient,
		GitClient:  f.GitClient,
		Config:     f.Config,
		Remotes:    f.Remotes,
		Branch:     f.Branch,
	}

	cmd := &cobra.Command{
		Use:   "status",
		Short: "Show status of relevant pull requests",
		Long: heredoc.Docf(`
			Show status of relevant pull requests.

			The status shows a summary of pull requests that includes information such as
			pull request number, title, CI checks, reviews, etc.

			To see more details of CI checks, run %[1]sgh pr checks%[1]s.
		`, "`"),
		Args: cmdutil.NoArgsQuoteReminder,
		RunE: func(cmd *cobra.Command, args []string) error {
			// support `-R, --repo` override
			opts.BaseRepo = f.BaseRepo
			opts.HasRepoOverride = cmd.Flags().Changed("repo")

			if runF != nil {
				return runF(opts)
			}
			return statusRun(opts)
		},
	}

	cmd.Flags().BoolVarP(&opts.ConflictStatus, "conflict-status", "c", false, "Display the merge conflict status of each pull request")
	cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.PullRequestFields)

	return cmd
}

func statusRun(opts *StatusOptions) error {
	ctx := context.Background()
	httpClient, err := opts.HttpClient()
Repository: cli/cli
File: pkg/cmdutil/args.go (Go)

Code before:
	}

	errMsg := fmt.Sprintf("unknown argument %q", args[0])
	if len(args) > 1 {
		errMsg = fmt.Sprintf("unknown arguments %q", args)
	}

	hasValueFlag := false
	cmd.Flags().Visit(func(f *pflag.Flag) {
		if f.Value.Type() != "bool" {
			hasValueFlag = true
		}
	})

	if hasValueFlag {
		errMsg += "; please quote all values that have spaces"
	}

	return FlagErrorf("%s", errMsg)
}

func GlobWindowsPaths(patterns []string) ([]string, error) {
	if runtime.GOOS != "windows" {
		return patterns, nil
	}
	var expansions []string
	for _, pattern := range patterns {
		matches, err := filepath.Glob(pattern)
		if err != nil {
			return nil, fmt.Errorf("%s: %s", pattern, err)
		}
		if len(matches) > 0 {
			expansions = append(expansions, matches...)
		} else {
			expansions = append(expansions, pattern)
		}
	}
	return expansions, nil
}


Review comment (performance):
Avoid unnecessary reallocations.
```
expansions := make([]string, 0, len(patterns)) // Preallocate slice
```

Code after:

	errMsg := fmt.Sprintf("unknown argument %q", args[0])
	if len(args) > 1 {
		errMsg = fmt.Sprintf("unknown arguments %q", args)
	}

	hasValueFlag := false
	cmd.Flags().Visit(func(f *pflag.Flag) {
		if f.Value.Type() != "bool" {
			hasValueFlag = true
		}
	})

	if hasValueFlag {
		errMsg += "; please quote all values that have spaces"
	}

	return FlagErrorf("%s", errMsg)
}

// Partition takes a slice of any type T and separates it into two slices
// of the same type based on the provided predicate function. Any item
// that returns true for the predicate will be included in the first slice
// returned, and any item that returns false for the predicate will be
// included in the second slice returned.
func Partition[T any](slice []T, predicate func(T) bool) ([]T, []T) {
	var matching, nonMatching []T
	for _, item := range slice {
		if predicate(item) {
			matching = append(matching, item)
		} else {
			nonMatching = append(nonMatching, item)
		}
	}
	return matching, nonMatching
}

// GlobPaths expands a list of file patterns into a list of file paths.
// If no files match a pattern, that pattern will return an error.
// If no pattern is passed, this returns an empty list and no error.
//
// For information on supported glob patterns, see
// https://pkg.go.dev/path/filepath#Match
func GlobPaths(patterns []string) ([]string, error) {
	expansions := []string{}

	for _, pattern := range patterns {
		matches, err := filepath.Glob(pattern)
		if err != nil {
			return nil, fmt.Errorf("%s: %v", pattern, err)
		}
Repository: cli/cli
File: pkg/cmdutil/args.go (Go)

Code before:
		errMsg = fmt.Sprintf("unknown arguments %q", args)
	}

	hasValueFlag := false
	cmd.Flags().Visit(func(f *pflag.Flag) {
		if f.Value.Type() != "bool" {
			hasValueFlag = true
		}
	})

	if hasValueFlag {
		errMsg += "; please quote all values that have spaces"
	}

	return FlagErrorf("%s", errMsg)
}

func GlobWindowsPaths(patterns []string) ([]string, error) {
	if runtime.GOOS != "windows" {
		return patterns, nil
	}
	var expansions []string
	for _, pattern := range patterns {
		matches, err := filepath.Glob(pattern)
		if err != nil {
			return nil, fmt.Errorf("%s: %s", pattern, err)
		}
		if len(matches) > 0 {
			expansions = append(expansions, matches...)
		} else {
			expansions = append(expansions, pattern)
		}
	}
	return expansions, nil
}


Review comment (bug):
```
fmt.Errorf("%s: %w", pattern, err) // Use %w for wrapped errors
```
Allows error wrapping, making debugging easier.

Code after:
	}

	hasValueFlag := false
	cmd.Flags().Visit(func(f *pflag.Flag) {
		if f.Value.Type() != "bool" {
			hasValueFlag = true
		}
	})

	if hasValueFlag {
		errMsg += "; please quote all values that have spaces"
	}

	return FlagErrorf("%s", errMsg)
}

// Partition takes a slice of any type T and separates it into two slices
// of the same type based on the provided predicate function. Any item
// that returns true for the predicate will be included in the first slice
// returned, and any item that returns false for the predicate will be
// included in the second slice returned.
func Partition[T any](slice []T, predicate func(T) bool) ([]T, []T) {
	var matching, nonMatching []T
	for _, item := range slice {
		if predicate(item) {
			matching = append(matching, item)
		} else {
			nonMatching = append(nonMatching, item)
		}
	}
	return matching, nonMatching
}

// GlobPaths expands a list of file patterns into a list of file paths.
// If no files match a pattern, that pattern will return an error.
// If no pattern is passed, this returns an empty list and no error.
//
// For information on supported glob patterns, see
// https://pkg.go.dev/path/filepath#Match
func GlobPaths(patterns []string) ([]string, error) {
	expansions := []string{}

	for _, pattern := range patterns {
		matches, err := filepath.Glob(pattern)
		if err != nil {
			return nil, fmt.Errorf("%s: %v", pattern, err)
		}
		if len(matches) == 0 {
			return []string{}, fmt.Errorf("no matches found for `%s`", pattern)
		}
		expansions = append(expansions, matches...)
Repository: cli/cli
File: pkg/cmd/project/item-edit/item_edit_test.go (Go)

Code before:
				"updateProjectV2ItemFieldValue": map[string]interface{}{
					"projectV2Item": map[string]interface{}{
						"ID": "item_id",
						"content": map[string]interface{}{
							"__typename": "Issue",
							"body":       "body",
							"title":      "title",
							"number":     1,
							"repository": map[string]interface{}{
								"nameWithOwner": "my-repo",
							},
						},
					},
				},
			},
		})

	client := queries.NewTestClient()

	ios, _, stdout, _ := iostreams.Test()
	ios.SetStdoutTTY(true)

	config := editItemConfig{
		io: ios,
		opts: editItemOpts{
			number:        123.45,
			numberChanged: true,
			itemID:        "item_id",
			projectID:     "project_id",
			fieldID:       "field_id",
		},
		client: client,
	}

	err := runEditItem(config)
	assert.NoError(t, err)
	assert.Equal(
		t,
		"Edited item \"title\"\n",
		stdout.String())
}

func TestRunItemEdit_Date(t *testing.T) {
	defer gock.Off()
	// gock.Observe(gock.DumpRequest)

	// edit item
	gock.New("https://api.github.com").
		Post("/graphql").
		BodyString(`{"query":"mutation UpdateItemValues.*","variables":{"input":{"projectId":"project_id","itemId":"item_id","fieldId":"field_id","value":{"date":"2023-01-01T00:00:00Z"}}}}`).
		Reply(200).

Review comment (suggestion):
**Change Requested**

Please add another test here that sets the `number: 0`. Currently that logic is not actually covered:

<img width="1459" alt="image" src="https://github.com/user-attachments/assets/2181b645-dc8b-46e4-850f-e97e9a1abe1a" />


Code after:
				"updateProjectV2ItemFieldValue": map[string]interface{}{
					"projectV2Item": map[string]interface{}{
						"ID": "item_id",
						"content": map[string]interface{}{
							"__typename": "Issue",
							"body":       "body",
							"title":      "title",
							"number":     1,
							"repository": map[string]interface{}{
								"nameWithOwner": "my-repo",
							},
						},
					},
				},
			},
		})

	client := queries.NewTestClient()

	ios, _, stdout, _ := iostreams.Test()
	ios.SetStdoutTTY(true)

	config := editItemConfig{
		io: ios,
		opts: editItemOpts{
			number:        123.45,
			numberChanged: true,
			itemID:        "item_id",
			projectID:     "project_id",
			fieldID:       "field_id",
		},
		client: client,
	}

	err := runEditItem(config)
	assert.NoError(t, err)
	assert.Equal(
		t,
		"Edited item \"title\"\n",
		stdout.String())
}

func TestRunItemEdit_NumberZero(t *testing.T) {
	defer gock.Off()
	// gock.Observe(gock.DumpRequest)

	// edit item
	gock.New("https://api.github.com").
		Post("/graphql").
		BodyString(`{"query":"mutation UpdateItemValues.*","variables":{"input":{"projectId":"project_id","itemId":"item_id","fieldId":"field_id","value":{"number":0}}}}`).
		Reply(200).
Repository: cli/cli
File: pkg/cmd/gist/edit/edit.go (Go)

Code before:
	// Remove a file from the gist
	if opts.RemoveFilename != "" {
		err := removeFile(gistToUpdate, opts.RemoveFilename)
		if err != nil {
			return err
		}

		return updateGist(apiClient, host, gistToUpdate)
	}

	filesToUpdate := map[string]string{}

	for {
		filename := opts.EditFilename
		candidates := []string{}
		for filename := range gistToUpdate.Files {
			candidates = append(candidates, filename)
		}

		sort.Strings(candidates)

		if filename == "" {
			if len(candidates) == 1 {
				filename = candidates[0]
			} else if len(candidates) == 0 {
				return errors.New("No file found in this gist.")
			} else {
				if !opts.IO.CanPrompt() {
					return errors.New("unsure what file to edit; either specify --filename or run interactively")
				}
				result, err := opts.Prompter.Select("Edit which file?", "", candidates)
				if err != nil {
					return fmt.Errorf("could not prompt: %w", err)
				}
				filename = candidates[result]
			}
		}

		gistFile, found := gistToUpdate.Files[filename]
		if !found {
			return fmt.Errorf("gist has no file %q", filename)
		}
		if shared.IsBinaryContents([]byte(gistFile.Content)) {
			return fmt.Errorf("editing binary files not supported")
		}

		var text string
		if src := opts.SourceFile; src != "" {
			if src == "-" {
				data, err := io.ReadAll(opts.IO.In)
				if err != nil {

Review comment (suggestion):
Please update the message to the suggestion below. It's because error messages should not be capitalised nor end with full stops.

```suggestion
				return errors.New("no file in the gist")
```

Code after:
	// Remove a file from the gist
	if opts.RemoveFilename != "" {
		err := removeFile(gistToUpdate, opts.RemoveFilename)
		if err != nil {
			return err
		}

		return updateGist(apiClient, host, gistToUpdate)
	}

	filesToUpdate := map[string]string{}

	for {
		filename := opts.EditFilename
		candidates := []string{}
		for filename := range gistToUpdate.Files {
			candidates = append(candidates, filename)
		}

		sort.Strings(candidates)

		if filename == "" {
			if len(candidates) == 1 {
				filename = candidates[0]
			} else if len(candidates) == 0 {
				return errors.New("no file in the gist")
			} else {
				if !opts.IO.CanPrompt() {
					return errors.New("unsure what file to edit; either specify --filename or run interactively")
				}
				result, err := opts.Prompter.Select("Edit which file?", "", candidates)
				if err != nil {
					return fmt.Errorf("could not prompt: %w", err)
				}
				filename = candidates[result]
			}
		}

		gistFile, found := gistToUpdate.Files[filename]
		if !found {
			return fmt.Errorf("gist has no file %q", filename)
		}
		if shared.IsBinaryContents([]byte(gistFile.Content)) {
			return fmt.Errorf("editing binary files not supported")
		}

		var text string
		if src := opts.SourceFile; src != "" {
			if src == "-" {
				data, err := io.ReadAll(opts.IO.In)
				if err != nil {