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

Add None value handling for JSONObject builder #2021

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

syncblaze
Copy link
Contributor

@syncblaze syncblaze commented Aug 17, 2024

Summary

If whe build a json object, the value should not be added to the json if its undefined type, but if its None it should just be added as None. These None checks were performed in the rest functions etc which used the JSON Object builder until now, but it makes far more sense to put this None check into the json builder.

Checklist

  • I have run nox and all the pipelines have passed.
  • I have made unittests according to the code I have added/modified/deleted.

Related issues

@beagold beagold added the enhancement New feature or request label Aug 28, 2024
Copy link
Contributor

@beagold beagold left a comment

Choose a reason for hiding this comment

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

Merging this as is could cause some slowdowns because of the double check for None that are lying around, would be nice to incorporate that into this pr too

hikari/internal/data_binding.py Outdated Show resolved Hide resolved
hikari/internal/data_binding.py Outdated Show resolved Hide resolved
hikari/internal/data_binding.py Outdated Show resolved Hide resolved
@@ -0,0 +1 @@
Add `None` check for every put function in `JSONObjectBuilder`
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for this changelog fragment, this change is purely internal

@beagold beagold added the skip-fragment-check Skip fragment checks for this PR as it doesnt need one label Aug 28, 2024
syncblaze and others added 3 commits August 28, 2024 13:28
Co-authored-by: beagold <86345081+beagold@users.noreply.github.com>
Signed-off-by: Blaze <88249929+syncblaze@users.noreply.github.com>
Co-authored-by: beagold <86345081+beagold@users.noreply.github.com>
Signed-off-by: Blaze <88249929+syncblaze@users.noreply.github.com>
Co-authored-by: beagold <86345081+beagold@users.noreply.github.com>
Signed-off-by: Blaze <88249929+syncblaze@users.noreply.github.com>
@syncblaze
Copy link
Contributor Author

Merging this as is could cause some slowdowns because of the double check for None that are lying around, would be nice to incorporate that into this pr too

i can do that no problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request skip-fragment-check Skip fragment checks for this PR as it doesnt need one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants