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

g.mapsets: Add JSON output #2542

Merged
merged 31 commits into from
Apr 11, 2024
Merged

g.mapsets: Add JSON output #2542

merged 31 commits into from
Apr 11, 2024

Conversation

cwhite911
Copy link
Contributor

Preparing module to add json output to g.mapsets.

@wenzeslaus wenzeslaus added this to In progress in OSGeo Community Sprint 2022 via automation Aug 27, 2022
@neteler neteler added this to the 8.3.0 milestone Aug 28, 2022
@neteler neteler added the enhancement New feature or request label Aug 28, 2022
@wenzeslaus wenzeslaus changed the title g.mapsets: fix indenting and added json output g.mapsets: Add JSON output Aug 28, 2022
general/g.mapsets/tests/test_g_mapsets_list_format.py Outdated Show resolved Hide resolved
general/g.mapsets/tests/test_g_mapsets_list_format.py Outdated Show resolved Hide resolved
general/g.mapsets/tests/test_g_mapsets_list_format.py Outdated Show resolved Hide resolved
@wenzeslaus wenzeslaus modified the milestones: 8.3.0, 8.4.0 Feb 10, 2023
@github-actions github-actions bot added Python Related code is in Python C Related code is in C module general labels Mar 21, 2024
general/g.mapsets/main.c Outdated Show resolved Hide resolved
general/g.mapsets/main.c Outdated Show resolved Hide resolved
general/g.mapsets/main.c Outdated Show resolved Hide resolved
general/g.mapsets/main.c Outdated Show resolved Hide resolved
general/g.mapsets/main.c Outdated Show resolved Hide resolved
general/g.mapsets/main.c Outdated Show resolved Hide resolved
echoix and others added 4 commits March 21, 2024 18:29
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added the HTML Related code is in HTML label Apr 4, 2024
general/g.mapsets/main.c Outdated Show resolved Hide resolved
general/g.mapsets/main.c Outdated Show resolved Hide resolved
cwhite911 and others added 2 commits April 5, 2024 12:39
Fixed typo.

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
general/g.mapsets/main.c Outdated Show resolved Hide resolved
general/g.mapsets/main.c Outdated Show resolved Hide resolved
general/g.mapsets/main.c Outdated Show resolved Hide resolved
cwhite911 and others added 5 commits April 7, 2024 14:57
Switch GUI section to Print

Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
Copy link
Contributor

@nilason nilason left a 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.

general/g.mapsets/list.c Outdated Show resolved Hide resolved
general/g.mapsets/list.c Outdated Show resolved Hide resolved
general/g.mapsets/list.c Outdated Show resolved Hide resolved
@cwhite911
Copy link
Contributor Author

Thanks for the feedback @nilason!

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.

I intentionally overloaded this with comments to kind of serve as documentation for parson library.

@nilason
Copy link
Contributor

nilason commented Apr 9, 2024

Thanks for the feedback @nilason!

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.

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
nilason previously approved these changes Apr 9, 2024
@nilason nilason self-requested a review April 9, 2024 19:42
@cwhite911
Copy link
Contributor Author

@nilason I removed some of the extra comments like you suggested and refactored the code to be dryer creating the json.

Copy link
Contributor

@nilason nilason left a comment

Choose a reason for hiding this comment

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

Great!

@echoix echoix dismissed wenzeslaus’s stale review April 10, 2024 21:37

Outdated and resolved

@echoix echoix merged commit 8bf604c into OSGeo:main Apr 11, 2024
26 checks passed
@wenzeslaus wenzeslaus removed their request for review April 22, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C docs enhancement New feature or request general HTML Related code is in HTML module Python Related code is in Python
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants