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.

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

fix: remove has assets condition #47

Merged
merged 2 commits into from
May 20, 2024
Merged

fix: remove has assets condition #47

merged 2 commits into from
May 20, 2024

Conversation

ReenigneArcher
Copy link
Contributor

@ReenigneArcher ReenigneArcher commented May 19, 2024

I don't think this is a bug in GitHub's API, but was perhaps a bug before. zipball and tarball "assets" are in a separate key as you can see here, and probably should have never been listed under "assets".

I also don't see a valid reason why you would use "assets" count to verify if it's a release or not. You already know it's a release because it's in the releases api.

P.S. It's unfortunate, but I'm pretty sure no one from GitHub will ever read anything in their community discussions: https://github.com/orgs/community/discussions/116306

@thadguidry
Copy link
Collaborator

Totally agree that we should not use "assets" count for verifying. I am only the maintainer, not the original developer. In fact, I don't develop on this project, but instead allow the community to contribute and review and test.

I'll merge your changes as it does seem like these fix the original underlying bug.

@thadguidry thadguidry merged commit 28771f0 into dev-drprasad:master May 20, 2024
@ReenigneArcher ReenigneArcher deleted the patch-1 branch July 13, 2024 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants