Skip to content

Conversation

@ascharpf28
Copy link

P1B: Starter Task: Refactoring PR

Use this pull request template to briefly answer the questions below in one to two sentences each.
Feel free to delete this text at the top after filling out the template.

You are not permitted to use generative AI services (e.g., ChatGPT) to compose the answers.
Any such use will be treated as a violation of academic integrity.

1. Issue

Link to the associated GitHub issue:
#114

Full path to the refactored file:
src/user/settings.js

What do you think this file does?
(Your answer does not have to be 100% correct; give a reasonable, evidence‑based guess.)
I think this file creates the settings for each user.

What is the scope of your refactoring within that file?
(Name specific functions/blocks/regions touched.)
I simplified the module.exports call to a function by pulling out functions inside of the function. I also fixed some repeated code by saving it as constants and reusing it.

Which Qlty‑reported issue did you address?
(Name the rule/metric and include the BEFORE value; e.g., “Cognitive Complexity 18 in render()”.)
Function with high complexity (count=23): exports

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?
The issue impacted the codebase's maintainability because it made it hard to read and understand what was going on.

What changes did you make to resolve the issue?
I organized it better and simplified it by taking functions outside of the module.export function to consolidate the code.

How do your changes improve maintainability? Did you consider alternatives?
It made the code easier to understand and analyze. I did not consider alternatives, I felt like the best way to simplify the function would be to take some of the bulky/bigger functions outside of it and call those functions within the module.export function.

3. Validation

How did you trigger the refactored code path from the UI?
I opened the site which automatically triggers the setting for each user.

Attach a screenshot of the logs and UI demonstrating the trigger.
(If you refactored a public/src/ file (front-end related file), watch logging via DevTools (Ctrl+Shift+I to open and then navigate to the 'Console' tab). If you refactored a src/ file, watch logging via ./nodebb log. Include the relevant UI view. Temporary logs should be removed before final commit.)
Screenshot 2026-01-23 at 8 39 16 PM

Attach a screenshot of qlty smells --no-snippets <full/path/to/file.js> showing fewer reported issues after the changes.
Screenshot 2026-01-23 at 1 13 21 PM

Screenshot 2026-01-23 at 1 13 12 PM

@coveralls
Copy link

Pull Request Test Coverage Report for Build 21307713212

Details

  • 63 of 65 (96.92%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 78.842%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/user/settings.js 63 65 96.92%
Totals Coverage Status
Change from base Build 21306090285: 0.001%
Covered Lines: 25362
Relevant Lines: 30332

💛 - Coveralls

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