Skip to content
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

fix: ensure statusCode is always an integer #439

Merged
merged 3 commits into from
Jul 10, 2024
Merged

fix: ensure statusCode is always an integer #439

merged 3 commits into from
Jul 10, 2024

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Jul 8, 2024

I have some bug, where the status code is set to a string value. One way to fix it, is by enforcing status to be always an integer.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 8, 2024

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 8, 2024

@gr2m
@wolfy1339

could you consider reviewing this please?

@wolfy1339
Copy link
Member

This only changes it on the error. Should there not be a middleware in @octokit/request instead

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 9, 2024

@wolfy1339

I am still investigating on the original issue. But this can be make it more resilient anyway. So I would still recommend the PR.

gr2m
gr2m approved these changes Jul 9, 2024
View reviewed changes
Copy link
Contributor

@gr2m gr2m left a comment

Choose a reason for hiding this comment

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

I'm okay with it, just would use isNan

src/index.ts Outdated Show resolved Hide resolved
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 10, 2024

@wolfy1339
@gr2m

Can we please merge it? THen I could check if this workaround works on gcf. :)

@wolfy1339 wolfy1339 merged commit 6eb8634 into octokit:main Jul 10, 2024
6 checks passed
@github-actions GitHub Actions
Copy link

🎉 This issue has been resolved in version 6.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Uzlopak Uzlopak deleted the string-status-code branch July 11, 2024 05:24
@Uzlopak Uzlopak mentioned this pull request Jul 11, 2024
Merged
@snyk-io snyk-io bot mentioned this pull request Aug 1, 2024
Open
@organich organich mentioned this pull request Sep 13, 2024
Open
@AKJUS AKJUS mentioned this pull request Sep 13, 2024
Open
@organich organich mentioned this pull request Sep 15, 2024
Open
@AKJUS AKJUS mentioned this pull request Sep 15, 2024
Open
@organich organich mentioned this pull request Sep 16, 2024
Open
@AKJUS AKJUS mentioned this pull request Sep 17, 2024
Open
@organich organich mentioned this pull request Sep 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wolfy1339 wolfy1339 wolfy1339 approved these changes

@gr2m gr2m gr2m approved these changes

Assignees
No one assigned
Labels
released
Projects
🧰 Octokit Active
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Uzlopak @wolfy1339 @gr2m