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

Sparse documentation? #133

Closed
yardasol opened this issue Oct 16, 2020 · 10 comments
Closed

Sparse documentation? #133

yardasol opened this issue Oct 16, 2020 · 10 comments

Comments

@yardasol
Copy link
Contributor

yardasol commented Oct 16, 2020

EDIT 1: The first comment below pointed out the language of the original issue had a bit of a critical and disrespectful tone. I generally agree and have updated the issue to reflect this.

The MOOSE code standards recommends that documentation be used often. Here is the example of the level of documentation expected from the MOOSE code standards for a .C file:

/**
 * The Kernel class is responsible for calculating the residuals for various
 * physics.
 *
 */
class Kernel
{
public:
  /**
   * This constructor should be used most often.  It initializes all internal
   * references needed for residual computation.
   *
   * @param system The system this variable is in
   * @param var_name The variable this Kernel is going to compute a residual for.
   */
  Kernel(System * system, std::string var_name);

  /**
   * This function is used to get stuff based on junk.
   *
   * @param junk The index of the stuff you want to get
   * @return The stuff associated with the junk you passed in
   */
  int returnStuff(int junk);

protected:
  /// This is the stuff this class holds on to.
  std::vector<int> stuff;
};

As far as I have seen (please feel free to correct me on this) Moltres documentation is not at this level of detail. I would recommend that this level of documentation be added to Moltres to better align with the MOOSE code standards as specified in the CONTRIBUTING.md document.

Use of a plugin in your code editor of choice for creating comments that can be used to generate Doxygen documentation will make this process more efficient.

This issue can be closed when Moltres code has been documented to a level that meets the MOOSE code standards.

Original issue preserved below

Original Title: Insufficient documentation throughout Moltres

Moltres documentation appears to be generally insufficient. As an open source project, well written and correctly used documentation enables new developers to easily familiarize themselves with the Moltres. This issue depends on the following:

  • Update the CONTRIBUTING.md document (Issue 132)
  • Create a Style Guide (Issue 131)

This issue can be closed once all code in the repository adheres to the specifications outlined in the new CONTRIBUTING.md document and the Style Guide.

@katyhuff
Copy link
Member

A contributing.md document exists and Moltres follows the MOOSE style guide and is not duplicated in this repo (because D.R.Y.). Please see my comments on the other issues with respect to these notes.

I understand that as a new user, you may feel confused by some of the documentation and I support improvements for clarity. I encourage those changes that you feel would be helpful. However, I also would like to take this moment to express that issues and PRs in this repository are part of the larger scope of professional engagements in ARFC over which the code of conduct applies. I'd like to request a more respectful and generous tone in such engagements to communicate due respect for the previous authors of this work, many of whom are your colleagues, one of whom is your advisor, and one of whom we are deeply grateful to for the sheer existence of Moltres. Entitling this issue "Insufficient documentation throughout Moltres" and characterizing the documentation situation as "generally insufficient" seems a bit baseless, given that moltres has been used by many students, including a complete stranger on another continent who required no assistance from us to incorporate Moltres simulations as a pivotal component of their PhD dissertation.

@katyhuff
Copy link
Member

Since this issue merely exists to refer to other subissues, I'd like to close it as duplicative. What do you think @yardasol?

@yardasol
Copy link
Contributor Author

I agree that this issue should be closed as duplicative.

@yardasol yardasol changed the title Insufficient documentation throughout Moltres Sparse documentation? Oct 16, 2020
@yardasol
Copy link
Contributor Author

yardasol commented Oct 16, 2020

Hmm, well now with these edits I think it is closer to being its own issue now, so maybe hold off on closing it just yet?

@yardasol
Copy link
Contributor Author

A contributing.md document exists and Moltres follows the MOOSE style guide and is not duplicated in this repo (because D.R.Y.). Please see my comments on the other issues with respect to these notes.

I understand that as a new user, you may feel confused by some of the documentation and I support improvements for clarity. I encourage those changes that you feel would be helpful. However, I also would like to take this moment to express that issues and PRs in this repository are part of the larger scope of professional engagements in ARFC over which the code of conduct applies. I'd like to request a more respectful and generous tone in such engagements to communicate due respect for the previous authors of this work, many of whom are your colleagues, one of whom is your advisor, and one of whom we are deeply grateful to for the sheer existence of Moltres. Entitling this issue "Insufficient documentation throughout Moltres" and characterizing the documentation situation as "generally insufficient" seems a bit baseless, given that moltres has been used by many students, including a complete stranger on another continent who required no assistance from us to incorporate Moltres simulations as a pivotal component of their PhD dissertation.

@katyhuff okay so I made some more edits and I think this issue is more appropriately worded now.

@katyhuff
Copy link
Member

Thanks!!

@smpark7
Copy link
Collaborator

smpark7 commented Oct 19, 2020

I've just briefly checked through a few of the kernel .h files and I noticed that some (e.g. GroupDiffusion.h) are missing documentation.

We can make a list of these files and make the appropriate edits to the documentation. In case you didn't know, here's a thorough breakdown of an example Moltres input file. If you're planning to work on this issue, you can ping me when you need help.

@yardasol
Copy link
Contributor Author

Good idea with making the list. I've made a textfile in the docum (short for documentation) branch of my arfc repo that has the names of all the files in src/ and include/ and is ready to be filled in with what changes or additions we should make for each file. I don't anticipate needing help on that, and it'll be a good opportunity to really get familiar with the overall structure of Moltres. Once I have gone through all the files in those directories and filled in what needs to be done, I'll ping here. Let me know if you think there are other places in the repo that I should put on the list.

@yardasol
Copy link
Contributor Author

unassigning myself from this because I'm not likely to get around to it any time soon and am less familiar with Moltres than some of the other people in the group.

@yardasol yardasol removed their assignment Oct 25, 2021
@smpark7
Copy link
Collaborator

smpark7 commented May 2, 2022

I'm closing this because the points here are covered by newer issues #179, #191, and #193

@smpark7 smpark7 closed this as completed May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants