8000 Use `delete` instead of `get`method for delete `ready to merge` label by Czaki · Pull Request #7160 · napari/napari · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use delete instead of getmethod for delete ready to merge label #7160

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 8, 2024
Merged

Use delete instead of getmethod for delete ready to merge label #7160

merged 1 commit into from
Aug 8, 2024

Conversation

Czaki
Copy link
Collaborator
@Czaki Czaki commented Aug 5, 2024

References and relevant issues

closes: #7159

documentation: https://docs.github.com/en/rest/issues/labels?apiVersion=2022-11-28#remove-a-label-from-an-issue

Description

@Czaki Czaki added the maintenance PR with maintance changes, label Aug 5, 2024
@Czaki Czaki added this to the 0.5.2 milestone Aug 5, 2024
@codecov Codecov
Copy link
codecov bot commented Aug 5, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.90%. Comparing base (698fd85) to head (3ac17f5).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #7160      +/-   ##
==========================================
- Coverage   92.99%   92.90%   -0.10%     
==========================================
  Files         619      619              
  Lines       56729    56729              
==========================================
- Hits        52757    52702      -55     
- Misses       3972     4027      +55

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lucyleeow
lucyleeow approved these changes Aug 7, 2024
View reviewed changes
Copy link
Contributor
@lucyleeow lucyleeow left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Actually, with delete if the label is not present, do you get the 404 error still?

@jni jni added the ready to merge Last chance for comments! Will be merged in ~24h label Aug 7, 2024
@Czaki
Copy link
Collaborator Author
Czaki commented Aug 7, 2024

Actually, with delete if the label is not present, do you get the 404 error still?

I do not remember which code, but I got an error response (4xx code)

@lucyleeow
Copy link
Contributor

I do not remember which code, but I got an error response (4xx code)

So would the response.raise_for_status still result in an error, even if you use delete?

@Czaki
Copy link
Collaborator Author
Czaki commented Aug 8, 2024

So would the response.raise_for_status still result in an error, even if you use delete?

No, because above, I check if label is already assigned to PR.

@lucyleeow lucyleeow merged commit 826fd59 into napari:main Aug 8, 2024
47 checks passed
@github-actions github-actions bot removed the ready 78D4 to merge Last chance for comments! Will be merged in ~24h label Aug 8, 2024
@Czaki Czaki deleted the fix-lable-removal branch August 8, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucyleeow lucyleeow lucyleeow approved these changes

Assignees
No one assigned
Labels
maintenance PR with maintance changes,
Projects
None yet
Milestone
0.5.2
Development

Successfully merging this pull request may close these issues.

Upgrade test constraints is failing to remove ready to merge label
3 participants
@Czaki @lucyleeow @jni
0