Implement PR closing and reopening #304

Merged
6543 merged 4 commits from YakoYakoYokuYoku/tea:pull-close-open into master 4 months ago

Fixes #301.

  • Try to not get a 403 as response, this likely means that the user is closing/reopening the PR as a repo owner instead of doing it as the PR owner as it should be expected. (See the upstream gitea issue.)
Fixes #301. - [x] ~~Try to not get a `403` as response, this likely means that the user is closing/reopening the PR as a repo owner instead of doing it as the PR owner as it should be expected.~~ (See the upstream [`gitea` issue](https://github.com/go-gitea/gitea/issues/14025).)
YakoYakoYokuYoku added 1 commit 4 months ago
7a4910f8ab
Implement pull request closing/reopening
YakoYakoYokuYoku changed title from Implement PR closing and reopening to WIP: Implement PR closing and reopening 4 months ago
lunny added the
kind/feature
label 4 months ago
lunny added this to the v0.7.0 milestone 4 months ago
6543 reviewed 4 months ago
Dismissed
// Copyright 2018 The Gitea Authors. All rights reserved.
6543 commented 4 months ago
Poster
Collaborator

2020

2020
noerw marked this conversation as resolved
noerw requested changes 4 months ago
Dismissed
cmd/pulls.go Outdated
Usage: "List, create, checkout and clean pull requests",
Description: `List, create, checkout and clean pull requests`,
Usage: "List, create, checkout, close, clean and reopen pull requests",
Description: `List, create, checkout, close, clean and reopen pull requests`,
noerw commented 4 months ago
Poster

This is becoming a bit long. Maybe Manage & checkout pull requests?

This is becoming a bit long. Maybe `Manage & checkout pull requests`?
YakoYakoYokuYoku marked this conversation as resolved
Collaborator

@YakoYakoYokuYoku can you apply changes of #291 to your pull ?

@YakoYakoYokuYoku can you apply changes of #291 to your pull ?
YakoYakoYokuYoku added 4 commits 4 months ago
ea0ad62e0d
Add feature comparison chart between forge CLIs (#294)
9c08becd75
replace flag globals, require context for commands (#291)
4a496c2108
Apply changes from #291
YakoYakoYokuYoku force-pushed pull-close-open from 4a496c2108 to 3c0a2acba2 4 months ago
YakoYakoYokuYoku requested review from noerw 4 months ago

@YakoYakoYokuYoku

  • Try to not get a 403 as response.

In my tests it works. You could show a more descriptive error in case it fails, though I think it's fine as it is.

  • Empty arguments so tea deletes/reopens the current PR.

I think this should be in another PR, as this requires detection what the current PR is. This is a feature that is generally neat to have (such as in tea open pull)

@YakoYakoYokuYoku > - [ ] Try to not get a `403` as response. In my tests it works. You could show a more descriptive error in case it fails, though I think it's fine as it is. > - [ ] Empty arguments so `tea` deletes/reopens the current PR. I think this should be in another PR, as this requires detection what the current PR is. This is a feature that is generally neat to have (such as in `tea open pull`)
noerw reviewed 4 months ago
Dismissed
func editPullState(ctx *cli.Context, opts gitea.EditPullRequestOption) error {
login, owner, repo := config.InitCommand(flags.GlobalRepoValue, flags.GlobalLoginValue, flags.GlobalRemoteValue)
if ctx.Args().Len() == 0 {
log.Fatal(ctx.Command.ArgsUsage)
noerw commented 4 months ago
Poster
- log.Fatal(ctx.Command.ArgsUsage)
+ return fmt.Errorf("Please provide a PR index")
```diff - log.Fatal(ctx.Command.ArgsUsage) + return fmt.Errorf("Please provide a PR index")
YakoYakoYokuYoku marked this conversation as resolved
Collaborator
  • Empty arguments so tea deletes/reopens the current PR.

I think this should be in another PR, as this requires detection what the current PR is. This is a feature that is generally neat to have (such as in tea open pull)

pull review commands will ned this too - something teaContext should ;)

@YakoYakoYokuYoku I agree with moving this into it's own pull

> > - [ ] Empty arguments so `tea` deletes/reopens the current PR. > > I think this should be in another PR, as this requires detection what the current PR is. This is a feature that is generally neat to have (such as in `tea open pull`) pull review commands will ned this too - something teaContext should ;) @YakoYakoYokuYoku I agree with moving this into it's own pull
Collaborator

@YakoYakoYokuYoku if you need help to apply latest changes to your pull - just ask

@YakoYakoYokuYoku if you need help to apply latest changes to your pull - just ask

I wasn't having any issues with merging commits, I was taking my time setting up my local Gitea instance for testing with this PR.

Testing revealed that I can close a PR, if I have permissions in the repo to do so. I suppose that this PR is for closing PRs as the repo owner, not the PR owner 🙃.

I wasn't having any issues with merging commits, I was taking my time setting up my local Gitea instance for testing with this PR. Testing revealed that I can close a PR, if I have permissions in the repo to do so. I suppose that this PR is for closing PRs as the repo owner, not the PR owner 🙃.
YakoYakoYokuYoku force-pushed pull-close-open from 3c0a2acba2 to 5810292885 4 months ago

Oh, now I get it & can reproduce. That's indeed an (upstream API) issue:
You should certainly be allowed to close your own PR.

I think we can merge this anyway and fix it upstream
https://github.com/go-gitea/gitea/issues/14025

Oh, now I get it & can reproduce. That's indeed an (upstream API) issue: You should certainly be allowed to close your own PR. I think we can merge this anyway and fix it upstream https://github.com/go-gitea/gitea/issues/14025
YakoYakoYokuYoku force-pushed pull-close-open from 5810292885 to ed9f5bbd4c 4 months ago

Since the PR 403 issue is not a blocker for this PR I also agree for merging.

Since the PR `403` issue is not a blocker for this PR I also agree for merging.
YakoYakoYokuYoku changed title from WIP: Implement PR closing and reopening to Implement PR closing and reopening 4 months ago
noerw approved these changes 4 months ago
Dismissed
noerw added the
status/needs-reviews
label 4 months ago
appleboy approved these changes 4 months ago
Dismissed
6543 removed the
status/needs-reviews
label 4 months ago
6543 merged commit a2e8b47c57 into master 4 months ago
6543 referenced this issue from a commit 4 months ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as a2e8b47c57.
Sign in to join this conversation.
Loading…
There is no content yet.