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

Suggestions for improving the documentation style #173

Closed
jbphet opened this issue Aug 23, 2023 · 1 comment
Closed

Suggestions for improving the documentation style #173

jbphet opened this issue Aug 23, 2023 · 1 comment
Assignees

Comments

@jbphet
Copy link
Contributor

jbphet commented Aug 23, 2023

Related to #165. I received some feedback in a code review a while back about inconsistent use of sentence fragments, punctuation, etc. See phetsims/number-line-operations#54. I'm seeing the same issue in the comments for this sim. Here is a short example. Note that the first comment is a fragment, which is fine, the second is a full sentence with capitalization, also fine. The third is a complete sentence written as a fragment.

  // the final mass and final proton number of a nuclide after a decay is done
  public readonly finalProtonNumberProperty: Property<number>;
  public readonly finalMassNumberProperty: Property<number>;

  // This cell (if available) holds all info about the initial mass and decay type.
  public readonly currentCellModelProperty: Property<NuclideChartCellModel | null>;

  public constructor( cellModelArray: NuclideChartCellModel[][], protonCountProperty: TReadOnlyProperty<number>, massNumberProperty: TReadOnlyProperty<number> ) {

    // keep track of the current nuclide cell to update the decay equation
    this.currentCellModelProperty = new Property( this.getCurrentCellModel( cellModelArray, protonCountProperty.value, massNumberProperty.value ) );

The inconsistencies in the style requires more cognitive effort for readers to interpret.

I made the changes that were suggested in phetsims/number-line-operations#54 and it took less time then I thought it would, and I feel that it improved the code. I know PhET has no standard for this, but I thought I'd put it out there for consideration.

@zepumph zepumph removed their assignment Aug 28, 2023
Luisav1 added a commit that referenced this issue Aug 29, 2023
…ntences. Shorten code to follow 120 column width reccomendation. See #176 and #173.
Luisav1 added a commit that referenced this issue Aug 29, 2023
…tences. Shorten code to follow 120 column width recommendation. See #176 and #173.
Luisav1 added a commit that referenced this issue Aug 29, 2023
…es. Shorten code to follow 120 column width recommendation. See #176 and #173.
Luisav1 added a commit that referenced this issue Aug 29, 2023
…s. Shorten code to follow 120 column width recommendation. See #176 and #173.
Luisav1 added a commit that referenced this issue Aug 29, 2023
… like sentences. Shorten code to follow 120 column width recommendation. See #176 and #173.
Luisav1 added a commit that referenced this issue Aug 30, 2023
…rten code to follow 120 column width recommendation. See #176 and #173.
Luisav1 added a commit that referenced this issue Aug 30, 2023
…orten code to follow 120 column width recommendation. See #176 and #173.
Luisav1 added a commit that referenced this issue Aug 30, 2023
…s. Shorten code to follow 120 column width recommendation. See #176 and #173.
@Luisav1
Copy link
Contributor

Luisav1 commented Aug 30, 2023

Thank @jbphet, that's a great suggestion. This was addressed in the commits above. Closing.

@Luisav1 Luisav1 closed this as completed Aug 30, 2023
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