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

Pickup latest release from cJSON #1047

Merged
merged 16 commits into from
Jul 31, 2023
Merged

Pickup latest release from cJSON #1047

merged 16 commits into from
Jul 31, 2023

Conversation

TingDaoK
Copy link
Contributor

@TingDaoK TingDaoK commented Jul 26, 2023

Issue #, if available:

Pick up change from https://github.com/DaveGamble/cJSON/releases/tag/v1.7.16

Description of changes:

  • Copy/pasted from the cJSON repo, and updated based on our lint.
  • To review, you can ignore the white space.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@TingDaoK TingDaoK changed the title Pickup cjson Pickup latest release from son Jul 26, 2023
@TingDaoK TingDaoK changed the title Pickup latest release from son Pickup latest release from CJson Jul 26, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (5c736d5) 82.23% compared to head (3fdba9a) 82.23%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1047   +/-   ##
=======================================
  Coverage   82.23%   82.23%           
=======================================
  Files          52       52           
  Lines        5641     5641           
=======================================
  Hits         4639     4639           
  Misses       1002     1002           
Files Changed Coverage Δ
source/encoding.c 74.78% <ø> (ø)
source/log_formatter.c 88.65% <ø> (ø)
source/logging.c 49.76% <ø> (ø)
source/memtrace.c 99.54% <ø> (ø)
source/posix/clock.c 90.00% <ø> (ø)
source/posix/system_info.c 20.88% <ø> (ø)
source/posix/thread.c 80.47% <ø> (ø)
source/process_common.c 87.87% <ø> (ø)
source/thread_shared.c 100.00% <ø> (ø)
source/allocator.c 97.36% <100.00%> (ø)
... and 7 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TingDaoK TingDaoK changed the title Pickup latest release from CJson Pickup latest release from cJSON Jul 26, 2023
Copy link
Contributor

@waahm7 waahm7 left a comment

Choose a reason for hiding this comment

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

Fix & Ship. I think we should just disable lint on these files and keep it similar to external cJson as much as possible.

source/external/cJSON.c Outdated Show resolved Hide resolved
source/external/cJSON.c Outdated Show resolved Hide resolved
/* helper for the cJSON_SetNumberValue macro */
CJSON_PUBLIC(double) cJSON_SetNumberHelper(cJSON *object, double number);
#define cJSON_SetNumberValue(object, number) ((object != NULL) ? cJSON_SetNumberHelper(object, (double)number) : (number)) //NOLINT
#define cJSON_SetNumberValue(object, number) (((object) != NULL) ? cJSON_SetNumberHelper((object), (double)(number)) : (number))
Copy link
Contributor

Choose a reason for hiding this comment

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

Brackets arount object seems redundant 🤔 Ignore if there is a reason for those

Suggested change
#define cJSON_SetNumberValue(object, number) (((object) != NULL) ? cJSON_SetNumberHelper((object), (double)(number)) : (number))
#define cJSON_SetNumberValue(object, number) ((object != NULL) ? cJSON_SetNumberHelper(object, (double)(number)) : (number))

Copy link
Contributor

Choose a reason for hiding this comment

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

Saw the lint errors, we should probably just disable lint on these files.

@@ -28,10 +28,10 @@ THE SOFTWARE.
* (3) Remove cJSON_GetErrorPtr and global_error as they are not thread-safe.
*/

/* clang-format off */
/* NOLINTBEGIN */

Copy link
Contributor

Choose a reason for hiding this comment

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

why are we reenabling clang format? if we disabling tidy, we should disable format as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is a .clang-format config that disabled the format already.

@TingDaoK TingDaoK merged commit 80f464e into main Jul 31, 2023
51 checks passed
@TingDaoK TingDaoK deleted the pickup-cjson branch July 31, 2023 20:13
Sign up for free to 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.

4 participants