Add `--remove-milestone` option to `issue edit` and `pr edit` #9344

babakks · 2024-07-21T13:18:25Z

This PR adds a --remove-milestone option to gh issue edit and gh pr edit commands.

Temporary note

Currently, the PR only adds the --remove-milestone option to the gh issue edit command, to avoid repetition. If/when maintainers approve the changes, then I'll replicate the same for gh pr edit.
Due to the default Git diff algorithm, changes made by the first two commits might show a bit strange diff. So, I recommend reviewing those two comments individually.

QA

(Since there's no CLI command to create milestones, I had to use the REST API.)

#!/usr/bin/env sh

_tmp=/tmp
_repo=gh-some-repo
_gh=$(pwd)/bin/gh
_pwd=$(pwd)

set -e
# Uncomment this to echo expanded commands:
#   set -x

cleanup () {
    cd $_pwd
    ARG=$?
    rm -rf "$_tmp/$_repo"
    gh repo delete --yes $_repo
    exit $ARG
} 
trap cleanup EXIT

cd $_tmp
$_gh repo create --private --add-readme $_repo
$_gh repo clone $_repo
cd $_repo

_owner="$($_gh repo view --json owner | jq -r .owner.login)"
# If `jq` is not available then use this instead (given that the owner login name
# has no special chars that need escaping when represented as JSON):
#  _owner="$($_gh repo view --json owner | sed -e 's/^.*"login":"//' -e 's/"\}\}$//')"

$_gh api \
  --silent \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  "/repos/$_owner/$_repo/milestones" \
   -f "title=foo" -f "state=open" -f "description=foo"

_issue="$($_gh issue create --title 'bar' --body '')"

# Assign milestone
$_gh issue edit --milestone foo "$_issue"
_result="$($_gh issue view --json milestone "$_issue")"
if [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected non-null milestone'
    exit 1
fi

# Unassign milestone
$_gh issue edit --remove-milestone "$_issue"
_result="$($_gh issue view --json  milestone "$_issue")"
if ! [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected null milestone'
    exit 1
fi

# Unassign milestone again, should return no error
$_gh issue edit --remove-milestone "$_issue"
_result="$($_gh issue view --json  milestone "$_issue")"
if ! [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected null milestone'
    exit 1
fi
echo 'PASS'

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

…he same time Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

andyfeller

Thanks for opening up this PR, @babakks! 🤗

Because this PR title mentions this being part of both gh issue edit and gh pr edit, I have some questions about how much of this should also be done in gh pr edit.

pkg/cmd/issue/edit/edit.go

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

…he same time Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

babakks · 2024-07-28T14:47:54Z

@andyfeller I've added the same --remove-milestone option to gh pr edit and pushed a bunch of commits.

QA

The following is the updated version of the above QA script, which now tests that the --remove-milestone option works with both gh issue edit and gh pr edit commands.

#!/usr/bin/env sh

_tmp=/tmp
_repo=gh-some-repo
_gh=$(pwd)/bin/gh
_pwd=$(pwd)

set -e
# Uncomment this to echo expanded commands:
#   set -x

cleanup () {
    cd $_pwd
    ARG=$?
    rm -rf "$_tmp/$_repo"
    gh repo delete --yes $_repo
    exit $ARG
} 
trap cleanup EXIT

cd $_tmp
$_gh repo create --private --add-readme $_repo
$_gh repo clone $_repo
cd $_repo

_owner="$($_gh repo view --json owner | jq -r .owner.login)"
# If `jq` is not available then use this instead (given that the owner login name
# has no special chars that need escaping when represented as JSON):
#  _owner="$($_gh repo view --json owner | sed -e 's/^.*"login":"//' -e 's/"\}\}$//')"

$_gh api \
  --silent \
  --method POST \
  -H "Accept: application/vnd.github+json" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  "/repos/$_owner/$_repo/milestones" \
   -f "title=foo" -f "state=open" -f "description=foo"

_issue="$($_gh issue create --title 'bar' --body '')"

# Assign issue to milestone
$_gh issue edit --milestone foo "$_issue"
_result="$($_gh issue view --json milestone "$_issue")"
if [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected non-null milestone for issue'
    exit 1
fi

# Unassign issue from milestone
$_gh issue edit --remove-milestone "$_issue"
_result="$($_gh issue view --json milestone "$_issue")"
if ! [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected null milestone for issue'
    exit 1
fi

# Unassign issue from milestone again, should return no error
$_gh issue edit --remove-milestone "$_issue"
_result="$($_gh issue view --json milestone "$_issue")"
if ! [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected null milestone for issue'
    exit 1
fi

# Create a new PR
git checkout -b pr-branch
touch new.md
git add new.md
git commit -m 'add new file'
git push origin pr-branch
_pr="$($_gh pr create --title 'foo' --body 'bar' --fill)"

# Assign PR to milestone
$_gh pr edit --milestone foo "$_pr"
_result="$($_gh pr view --json milestone "$_pr")"
if [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected non-null milestone for PR'
    exit 1
fi

# Unassign PR from milestone
$_gh pr edit --remove-milestone "$_pr"
_result="$($_gh pr view --json milestone "$_pr")"
if ! [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected null milestone for PR'
    exit 1
fi

# Unassign PR from milestone again, should return no error
$_gh pr edit --remove-milestone "$_pr"
_result="$($_gh pr view --json milestone "$_pr")"
if ! [ "$_result" = '{"milestone":null}' ]; then
    echo 'FAIL: expected null milestone for PR'
    exit 1
fi
echo 'PASS'

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

babakks · 2024-07-29T20:43:29Z

@andyfeller I noticed the opts.RemoveMilestone field that I'd added has no use inside the editRun method. So, I treated it like the bodyFile var, which its scope is closure-bounded.

andyfeller

✨ thank you for continuing to make gh a little bit better every day, @babakks ❤️

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://togithub.com/cli/cli) | minor | `v2.53.0` -> `v2.54.0` | --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.54.0`](https://togithub.com/cli/cli/releases/tag/v2.54.0): GitHub CLI 2.54.0 [Compare Source](https://togithub.com/cli/cli/compare/v2.53.0...v2.54.0) #### What's Changed - Remove redundant whitespace by [@jessehouwing](https://togithub.com/jessehouwing) in [https://github.com/cli/cli/pull/9334](https://togithub.com/cli/cli/pull/9334) - Remove attestation test that requires being online by [@steiza](https://togithub.com/steiza) in [https://github.com/cli/cli/pull/9340](https://togithub.com/cli/cli/pull/9340) - Update documentation for gh api PATCH by [@cmbuckley](https://togithub.com/cmbuckley) in [https://github.com/cli/cli/pull/9352](https://togithub.com/cli/cli/pull/9352) - Clarify usage of template flags for PR and issue creation by [@williammartin](https://togithub.com/williammartin) in [https://github.com/cli/cli/pull/9354](https://togithub.com/cli/cli/pull/9354) - Expose json databaseId field for release commands by [@williammartin](https://togithub.com/williammartin) in [https://github.com/cli/cli/pull/9356](https://togithub.com/cli/cli/pull/9356) - Expose fullDatabaseId for PR json export by [@williammartin](https://togithub.com/williammartin) in [https://github.com/cli/cli/pull/9355](https://togithub.com/cli/cli/pull/9355) - Handle `--bare` clone targets by [@hyperrealist](https://togithub.com/hyperrealist) in [https://github.com/cli/cli/pull/9271](https://togithub.com/cli/cli/pull/9271) - Slightly clarify when CLI exits with code 4 by [@williammartin](https://togithub.com/williammartin) in [https://github.com/cli/cli/pull/9358](https://togithub.com/cli/cli/pull/9358) - Update sigstore-go in gh CLI to v0.5.1 by [@steiza](https://togithub.com/steiza) in [https://github.com/cli/cli/pull/9366](https://togithub.com/cli/cli/pull/9366) - Exit with 1 on authentication issues by [@Stausssi](https://togithub.com/Stausssi) in [https://github.com/cli/cli/pull/9240](https://togithub.com/cli/cli/pull/9240) - build(deps): bump github.com/gabriel-vasile/mimetype from 1.4.4 to 1.4.5 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/cli/cli/pull/9372](https://togithub.com/cli/cli/pull/9372) - build(deps): bump github.com/google/go-containerregistry from 0.20.0 to 0.20.1 by [@dependabot](https://togithub.com/dependabot) in [https://github.com/cli/cli/pull/9373](https://togithub.com/cli/cli/pull/9373) - Add `--remove-milestone` option to `issue edit` and `pr edit` by [@babakks](https://togithub.com/babakks) in [https://github.com/cli/cli/pull/9344](https://togithub.com/cli/cli/pull/9344) - handle attest case insensitivity by [@ejahnGithub](https://togithub.com/ejahnGithub) in [https://github.com/cli/cli/pull/9392](https://togithub.com/cli/cli/pull/9392) #### New Contributors - [@cmbuckley](https://togithub.com/cmbuckley) made their first contribution in [https://github.com/cli/cli/pull/9352](https://togithub.com/cli/cli/pull/9352) - [@hyperrealist](https://togithub.com/hyperrealist) made their first contribution in [https://github.com/cli/cli/pull/9271](https://togithub.com/cli/cli/pull/9271) - [@Stausssi](https://togithub.com/Stausssi) made their first contribution in [https://github.com/cli/cli/pull/9240](https://togithub.com/cli/cli/pull/9240) - [@ejahnGithub](https://togithub.com/ejahnGithub) made their first contribution in [https://github.com/cli/cli/pull/9392](https://togithub.com/cli/cli/pull/9392) **Full Changelog**: cli/cli@v2.53.0...v2.54.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/izumin5210/dotfiles).  --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: izumin5210-update-aqua-checksum[bot] <169593670+izumin5210-update-aqua-checksum[bot]@users.noreply.github.com>

williammartin · 2024-09-18T10:51:31Z

@babakks just randomly I was looking at your script here and I wondered whether I was missing something about the cleanup function:

cleanup () {
    cd $_pwd
    ARG=$?
    rm -rf "$_tmp/$_repo"
    gh repo delete --yes $_repo
    exit $ARG
}

Should ARG=$? go before the cd? Otherwise it seems like the exit code will be the exit code of the cd.

babakks · 2024-09-18T11:09:33Z

@williammartin I just tested, and you're right. My bad. Thanks.

babakks added 5 commits July 21, 2024 12:45

Verify --body and --body-file are not assignable at the same time

17257df

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Remove unused expected output from test case (with wantsErr: true)

9dabc3b

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Add --remove-milestone option

683576d

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Assert correct parsing of --remove-milestone option

168a883

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Verify --milestone and --remove-milestone are not assignable at t…

42da8ba

…he same time Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

babakks requested a review from a team as a code owner July 21, 2024 13:18

babakks requested a review from andyfeller July 21, 2024 13:18

cliAutomation added the external pull request originating outside of the CLI core team label Jul 21, 2024

andyfeller reviewed Jul 22, 2024

View reviewed changes

pkg/cmd/issue/edit/edit.go Outdated Show resolved Hide resolved

pkg/cmd/issue/edit/edit.go Outdated Show resolved Hide resolved

pkg/cmd/issue/edit/edit.go Outdated Show resolved Hide resolved

babakks added 7 commits July 28, 2024 13:27

Merge branch 'trunk' into 9292-add-remove-milestone-option

9362143

Point to Editable.MilestoneId method

6e34e81

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Improve --remove-milestone option description

4c8ec84

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Add --remove-milestone option

1520292

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Verify --body and --body-file are not assignable at the same time

6dc8cc4

Assert correct parsing of --remove-milestone option

605b4b1

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Verify --milestone and --remove-milestone are not assignable at t…

c9fb4ed

…he same time Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

babakks requested a review from andyfeller July 28, 2024 14:48

babakks added 4 commits July 29, 2024 21:22

Merge branch 'trunk' into 9292-add-remove-milestone-option

457274b

Use closure-scoped variable to catch --remove-milestone option

842562d

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Use closure-scoped variable to catch --remove-milestone option

a73ae65

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

Fix missing variable

58e6196

Signed-off-by: Babak K. Shandiz <babak.k.shandiz@gmail.com>

andyfeller approved these changes Jul 29, 2024

View reviewed changes

andyfeller merged commit b05bddc into cli:trunk Jul 29, 2024

babakks deleted the 9292-add-remove-milestone-option branch July 30, 2024 08:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `--remove-milestone` option to `issue edit` and `pr edit` #9344

Add `--remove-milestone` option to `issue edit` and `pr edit` #9344

Uh oh!

babakks commented Jul 21, 2024 •

edited

Loading

Uh oh!

andyfeller left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

babakks commented Jul 28, 2024

Uh oh!

babakks commented Jul 29, 2024

Uh oh!

andyfeller left a comment

Uh oh!

williammartin commented Sep 18, 2024

Uh oh!

babakks commented Sep 18, 2024

Uh oh!

Uh oh!

Add --remove-milestone option to issue edit and pr edit #9344

Add --remove-milestone option to issue edit and pr edit #9344

Uh oh!

Conversation

babakks commented Jul 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Temporary note

QA

Uh oh!

andyfeller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

babakks commented Jul 28, 2024

QA

Uh oh!

babakks commented Jul 29, 2024

Uh oh!

andyfeller left a comment

Choose a reason for hiding this comment

Uh oh!

williammartin commented Sep 18, 2024

Uh oh!

babakks commented Sep 18, 2024

Uh oh!

Uh oh!

Add `--remove-milestone` option to `issue edit` and `pr edit` #9344

Add `--remove-milestone` option to `issue edit` and `pr edit` #9344

babakks commented Jul 21, 2024 •

edited

Loading