-
-
Notifications
You must be signed in to change notification settings - Fork 327
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
g.mapsets: Add JSON output #2542
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Fixed typo. Co-authored-by: Anna Petrasova <[email protected]>
Switch GUI section to Print Co-authored-by: Anna Petrasova <[email protected]>
Co-authored-by: Anna Petrasova <[email protected]>
Co-authored-by: Anna Petrasova <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greatly appreciate your work with JSON output!
I added some comments/suggestions.
A more general suggestion is to limit comments to where they really are helpful, not commenting every step in the code. If the code is good written it is usually self-explanatory.
Co-authored-by: Nicklas Larsson <[email protected]>
Co-authored-by: Nicklas Larsson <[email protected]>
Thanks for the feedback @nilason!
I intentionally overloaded this with comments to kind of serve as documentation for parson library. |
If parson library would have a cryptic API with acronyms of dropped vowels I would perhaps agree with the need for that. Parson API is however very descriptive, e.g.: // Serialize root object to string and print it to stdout
serialized_string = json_serialize_to_string_pretty(root_value);
puts(serialized_string); ...the comment does not add anything new compared to just reading the code. That is what I mean by self-explanatory code. Another example: // Free memory
json_free_serialized_string(serialized_string);
json_value_free(root_value); We shouldn't underestimate our potential contributors, I would presume there is at the very minimum a basic knowledge of C. The code without the comments, would perfectly work as a good example. |
@nilason I removed some of the extra comments like you suggested and refactored the code to be dryer creating the json. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
Preparing module to add json output to g.mapsets.