search

aconchillo / guile-json

JSON module for Guile
GNU General Public License v3.0
100 stars 34 forks source link

add a case for building the JSON of empty JSON objects #45

Closed ZelphirKaltstahl closed 5 years ago

ZelphirKaltstahl commented 5 years ago

This prevents the bug described in https://github.com/aconchillo/guile-json/issues/44.

By adding an additional case, the throwing of an exception is prevented and instead the empty JSON object is correctly build in the problematic case.

aconchillo commented 5 years ago

thank you! could you please combine all 3 commits into a single one?

ZelphirKaltstahl commented 5 years ago

Hi Aleix!

I tried to squash the commits into one commit and edited the commit message to reflect all changes. However, it does not show up on this page as only one commit now and also an additional merge commit had to be created, otherwise my git push to the fork's master branch was rejected. Now I am not sure I did this squashing thing correctly, as there is now that merge commit (https://github.com/ZelphirKaltstahl/guile-json/commit/154699c8c9c05cb5b4ee859ace486eae002ee82c which contains no changes o_0) on top of what I wanted to be the last commit. I am doing this squashing thing for the first time.

Is it correct now?

It is little code so I could also recreate the whole fork and do it again in one commit, if necessary.

Edit: I removed the empty merge commit now, but when I look at the "commits" of this PR (at the top, there is a tab with the label "commits"), I see an old commit plus the new one which contains 3 commits which have been squashed into 1 commit: https://github.com/aconchillo/guile-json/pull/45/commits

aconchillo commented 5 years ago

doesn't look correct. yes, a new PR seems ok.