Skip to content

Return json.Unmarshal error when importing issues deferred - #2892

Merged
gmlewis merged 2 commits into
google:masterfrom
nikpivkin:fix/issue-import-202
Aug 22, 2023
Merged

Return json.Unmarshal error when importing issues deferred#2892
gmlewis merged 2 commits into
google:masterfrom
nikpivkin:fix/issue-import-202

Conversation

@nikpivkin

Copy link
Copy Markdown
Contributor

The error was not returned when the 202 status was received during the issues import.

@codecov

codecovBot commented Aug 21, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2892 (48a5fb2) into master (a0e8f35) will increase coverage by 0.04%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

@@ Coverage Diff @@## master #2892 +/- ##
==========================================
+ Coverage 98.06% 98.10% +0.04% 
==========================================
Files 140 142 +2 Lines 12269 12340 +71 ==========================================
+ Hits 12031 12106 +75 + Misses 162 159 -3 + Partials 76 75 -1 
Files ChangedCoverage Δ
github/issue_import.go100.00% <100.00%> (+7.54%)⬆️

... and 3 files with indirect coverage changes

@gmlewisgmlewis changed the title fix: return error when importing issues deferredReturn json.Unmarshal error when importing issues deferredAug 21, 2023

@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, @nikpivkin !
Could you please add a test case that demonstrates the problem?

In the past, I've found it really handy to demonstrate json.Unmarshal errors by forcing the data to be something bogus for JSON like {].

@nikpivkin

Copy link
Copy Markdown
ContributorAuthor

@gmlewis Hi! Fixed! The initial fix was that the error did not return when the response status was 202. (nil returned)

@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, @nikpivkin !
LGTM.

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

@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @gabriel-samfira !
Merging.

@gmlewis
gmlewis merged commit 38ca69f into google:masterAug 22, 2023
gmlewis pushed a commit to gmlewis/go-github that referenced this pull request Sep 19, 2023
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.

3 participants

@nikpivkin@gmlewis@gabriel-samfira