Skip to content

Add more documentation for some components#410

Open
Hanbing59 wants to merge 1 commit intogroops-devs:mainfrom
Hanbing59:pr-docfix
Open

Add more documentation for some components#410
Hanbing59 wants to merge 1 commit intogroops-devs:mainfrom
Hanbing59:pr-docfix

Conversation

@Hanbing59
Copy link
Copy Markdown
Contributor

No description provided.

@tmayerguerr
Copy link
Copy Markdown
Member

Hi Hanbing,

I really appreciate your contributions. You've already made GROOPS better in so many ways. But to be honest, I have some issues with this pull request. Right now, I wouldn't want to implement most of the proposed changes.

  • Please avoid traling whitespaces as they create differences in the files (additional commits) without actually changing anything. You can check this with git diff --check.

  • It seems more sensible to me to include detailed descriptions of the input data in the user documentation (docstring) rather than in the source code documentation.

  • Please try to avoid using external URLs that are likely to become outdated quickly (e.g. EOP for ITRF2020 update 2024, EOP_C04_20u24).

  • config.h: Adding all @param without explanations that aid understanding only bloats the files.

  • source/programs/conversion/eop/iersRapidIAU2000EarthOrientationParameter.cpp: IAU1980-baed EOP data is NOT supported even if it uses the same format.

By the way, would you be interested in chatting over Zoom or WebEx? I always find it easier to talk face-to-face.
If so, just send me an email (mayer-guerr(a)tugraz.at) to schedule an appointment sometime; if not, that’s fine with me, of course.

Best regards
Torsten

@Hanbing59
Copy link
Copy Markdown
Contributor Author

Hi Torsten,

thanks for your kind words and detailed comments. Regarding to your concerns:

  • Please avoid traling whitespaces as they create differences in the files (additional commits) without actually changing anything. You can check this with git diff --check.

Oh, that's something I didn't expected and realized. Sorry for inconvenience that may have caused. I will keep that in mind in my following PRs. Just wondering, is it OK to set git or github to ignore differences caused only by white-paces or blank lines? I am not very sure about that, just out of curiosity.

  • It seems more sensible to me to include detailed descriptions of the input data in the user documentation (docstring) rather than in the source code documentation.

I agree with you that, the docstring should always be the place where documentation on usages should be placed. The reasons I did not put much doc there are: (1) I did not get very familiar yet about how docstrings work in GROOPS, e.g. different labels, commands (except for LaTex ones) within inside and their interactions with the final output html and LaTex files; (2) it seems that, only by re-compiling, we could have a preview of effects on output html or pdf documentation files caused by modifications on docstrings. Then, it would be cumbersome to check modifications frequently.

However, except for docstrings, I think documentations about interfaces, declarations (something called API documentation?) may be also necessary whenever adding them is not too much burden. Just share my personal thoughts.

  • Please try to avoid using external URLs that are likely to become outdated quickly (e.g. EOP for ITRF2020 update 2024, EOP_C04_20u24).

Agree. I just found it is not easy to choose long-lasting while as precise as possible references (precise means, exactly input files needed for programs). Maybe you have better ideals about this.

  • config.h: Adding all @param without explanations that aid understanding only bloats the files.

Similar thoughts as above, add API docs whenever possible. Empty explanations just mean, no precise understanding about the code available yet but would love to add afterwards.

  • source/programs/conversion/eop/iersRapidIAU2000EarthOrientationParameter.cpp: IAU1980-baed EOP data is NOT supported even if it uses the same format.

Yes. Just wanted to document the full capability, as the conversion may be used for other purpose outside of GROOPS. Not sure if it makes sense from your point.

By the way, would you be interested in chatting over Zoom or WebEx? I always find it easier to talk face-to-face. If so, just send me an email (mayer-guerr(a)tugraz.at) to schedule an appointment sometime; if not, that’s fine with me, of course.
Sure, I'll send you an e-mail.

Thanks again for your patient reply!

Best regards,
Hanbing

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.

2 participants