Repository navigation
Fixes missing "form" variable declaration. - #8797
Conversation
|
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes. |
|
Review ETA: Thursday |
|
Review ETA: 7pm 9/22/2026 |
|
Review ETA: 7pm 9/23/2026 |
Tomomi-K1
left a comment
There was a problem hiding this comment.
Great job and thank you for taking the issue!
- I agree with your solution to add "const" in front of form
- branches are correct
- tested locally, submitting form worked
nit pick: It's anything that stops approval 👍
- in issue page, I would check the box "In a comment in this issue, add your analysis and recommendations...."
- in the pull request, I would delete the first paragraph "Please note: You must be a member of the HFLA website team in order to create pull requests. Please see our page on how to join us as a member at HFLA: https://www.hackforla.org/getting-started. Delete this message if you joined this team via onboarding."
|
Review ETA: 9/23 EOD |
|
Review ETA: 09/25/2026 EOD |
anthonylo87
left a comment
There was a problem hiding this comment.
Hi @cchrizzle,
Thanks for working on this issue, I've done my review - only thing I would modify is in the "What Changes Did You Make" section, include some of the details from your analysis on the parent issue (so I don't have to go back and forth between the issue/PR to understand what's going on)
i.e. "Fixed missing form variable declaration by adding const (variable is not reassigned within function)"
Other than that, everything looks great - issue linked correctly, branches set up correct, and no CodeQL alerts. Approved on my end!
computarisis
left a comment
There was a problem hiding this comment.
Hi @cchrizzle! I've gone thorugh the details, and things looked good from my end. You correctly added 'const' to make the variable local and addess the warning; and you correctly went through the usual protocol for branching and filling the templates.
ldaws003
left a comment
There was a problem hiding this comment.
Hi @cchrizzle, good work with this issue. The added keyword fixes the quality issue and keeps it inline with the how the other form variables are declared. Approved.
|
@egcuriel I'm going to merge this as it has enough reviews, there are a couple other Medium PRs open right now you can review if you need to for your skills issue. |
|
@Tomomi-K1 & @anthonylo87 Appreciate both of your recommendations, went ahead and implemented them. Thank you! |
Fixes #6668
What changes did you make?
formvariable declaration by addingconst(variable is not reassigned within function) based on original analysis:Why did you make the changes (we will use this info to test)?
CodeQL Alerts
After the PR has been submitted and the resulting GitHub actions/checks have been completed, developers should check the PR for CodeQL alert annotations.
Check the PR's comments. If present on your PR, the CodeQL alert looks similar as shown
Please let us know that you have checked for CodeQL alerts. Please do not dismiss alerts.
Instructions for resolving CodeQL alerts
If CodeQL alert/annotations appear, refer to How to Resolve CodeQL alerts.
In general, CodeQL alerts should be resolved prior to PR reviews and merging
Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)