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

chore: corrected typos in routes comments #3840

Merged
merged 1 commit into from
Jun 24, 2023
Merged

chore: corrected typos in routes comments #3840

merged 1 commit into from
Jun 24, 2023

Conversation

JamesShaver
Copy link
Contributor

Corrected typos in the comments

Fixes #0000

Changes proposed in this pull request:

Reviewers should focus on:

Screenshot

QA

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)
  • Related core extension PRs: (Remove if irrelevant)

Corrected typos in the comments
@JamesShaver JamesShaver requested a review from a team as a code owner June 22, 2023 00:14
Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Do we need all these comments? Feels like the code is fairly self explanatory

@JamesShaver
Copy link
Contributor Author

Do we need all these comments? Feels like the code is fairly self explanatory

The code comments show up in IDEs, so I would say yes each method needs to be commented accurately.

@davwheat
Copy link
Member

If they were function definitions, sure. I don't oppose to any of these, though.

@SychO9 SychO9 changed the title Update routes.php chore: corrected typos in routes comments Jun 24, 2023
Copy link
Member

@SychO9 SychO9 left a comment

Choose a reason for hiding this comment

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

Thanks! I do agree with Sasha though, the code is readable enough not to need these comments.

@SychO9 SychO9 merged commit cf70865 into flarum:2.x Jun 24, 2023
@JamesShaver JamesShaver deleted the patch-1 branch June 25, 2023 04:35
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.

4 participants