Skip to content

feat: Add support for sub-issue - #3580

Merged
gmlewis merged 17 commits into
google:masterfrom
e7217:feat/add-subissue
May 26, 2025
Merged

feat: Add support for sub-issue#3580
gmlewis merged 17 commits into
google:masterfrom
e7217:feat/add-subissue

Conversation

@e7217

@e7217e7217 commented May 20, 2025

Copy link
Copy Markdown
Contributor

Hello,
I have added some features to support Sub-issue.

The following features have been added:

  • Add sub-issue
  • Remove sub-issue
  • Get sub-issue list by issue
  • Reprioritize sub-issue

Sorry for the delay in developing this feature.
Thank you for your review😀

Fixes#3546
cc@gmlewis

@codecov

codecovBot commented May 20, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.32%. Comparing base (304d3d0) to head (fde3e4b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@ Coverage Diff @@## master #3580 +/- ##
==========================================
+ Coverage 91.29% 91.32% +0.03% 
==========================================
Files 183 184 +1 Lines 16087 16143 +56 ==========================================
+ Hits 14686 14742 +56 
Misses 1227 1227 Partials 174 174 

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @e7217! Please follow the instructions in step 4 of CONTRIBUTING.md and push the changes to this PR.

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @e7217!
This was a quick review because that's all I currently have time for, but should help move this PR in the right direction.

Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue_test.go Outdated
Comment threadgithub/sub_issue_test.go
Comment threadgithub/sub_issue.go
e7217and others added 9 commits May 21, 2025 01:59
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@e7217

Copy link
Copy Markdown
ContributorAuthor

Thank you, @e7217! This was a quick review because that's all I currently have time for, but should help move this PR in the right direction.

@gmlewis I appreciate your review. It has been helpful to me.

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @e7217!
Just a few more tweaks, and a few comments that were not addressed previously, please... then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.

Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
Comment threadgithub/sub_issue.go Outdated
@gmlewisgmlewis added the NeedsReview PR is awaiting a review before merging. label May 20, 2025
e7217and others added 2 commits May 21, 2025 08:49
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
@e7217

Copy link
Copy Markdown
ContributorAuthor

@gmlewis
Oh, it was my mistake. I'm sorry for the inconvenience. Despite that, thank you for your detailed review. I have made a commit that includes your checking points and updated the test cases.

Comment threadgithub/sub_issue.go Outdated
Comment on lines +60 to +61
// TODO: remove custom Accept header when this API fully launches.
req.Header.Set("Accept", mediaTypeV3)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking at the documentation for these endpoints, I'm not seeing any need to add the mediaTypeV3 header, so all these lines can safely be removed, I believe. Feel free to correct me if I've missed them.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Looking at the documentation for these endpoints, I'm not seeing any need to add the mediaTypeV3 header, so all these lines can safely be removed, I believe. Feel free to correct me if I've missed them.

The header values are included as shown below, and I used the predefined header values from go-github. Since the response was coming back correctly, I applied them as is. If you think it would be better to remove this, I will delete that part. Please let me know your opinion.
image

@gmlewis

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right, but the one you added doesn't match.

You added application/vnd.github.v3+json, so I think this one can be removed.

@e7217e7217May 21, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

@gmlewis ok, I have removed it. Thank you!

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @e7217!
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

@stevehipwell - might you have time for a code review? Thank you!

@DiegoDev2DiegoDev2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @DiegoDev2!
Merging.

@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label May 26, 2025
@gmlewis
gmlewis merged commit cc0e642 into google:masterMay 26, 2025
@e7217
e7217 deleted the feat/add-subissue branch May 27, 2025 05:05
jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for Sub-Issue endpoints

3 participants

@e7217@gmlewis@DiegoDev2