Skip to content

Conversation

@ShreyasHariCMU
Copy link

@ShreyasHariCMU ShreyasHariCMU commented Jan 24, 2026

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:
#167

Full path to the refactored file:
public/src/app.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 is the main client-side/front-end file that helps NodeBB run in the browser. It sets up common stuff like loading scripts/features and provides helper functions that other front-end code uses.

What is the scope of your refactoring within that file?
(Name specific functions/blocks/regions touched.)

  • app.parseAndTranslate

Which Qlty‑reported issue did you address?
(Name the rule/metric and include the BEFORE value; e.g., “Cognitive Complexity 18 in render()”.)

  • 330 Function with many parameters (count = 4): parseAndTranslate

2. Refactoring

How did the specific issue you chose impact the codebase’s maintainability?

  • The specific issue I chose impacted the codebase's maintainability because parseAndTranslate took 4 separate arguments, which was confusing to call correctly and easy to accidentally pass the wrong thing.

What changes did you make to resolve the issue?

  • I changed parseAndTranslate so you can pass data and blockName in a single options object, and it still supports the old call style

How do your changes improve maintainability? Did you consider alternatives?

  • This makes the function easier to read and use because the inputs are grouped together and its less likely someone passes the arguments in the wrong order.
  • Maybe a better fix would have been to introduce a small wrapper that uses an options object and call that.

3. Validation

How did you trigger the refactored code path from the UI?

  • I first added console.log('Shreyas Hari!') to our parseAndTranslate function.
  • On the NodeBB homepage, clicked on the "Popular" tab and selected "New Topics" from the dropdown provided.
  • The page refreshed and the console.log message "Shreyas Hari!" appeared in the DevTools Console, confirming our refactored function was run.

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 02 02 PM
Screenshot 2026-01-23 at 8 36 07 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 8 37 21 PM

@ShreyasHariCMU ShreyasHariCMU changed the title Refactor parseAndTranslate to reduce parameters Refactor( public/src/app.js): reduce parameters in parseAndTranslate Jan 24, 2026
@ShreyasHariCMU ShreyasHariCMU changed the title Refactor( public/src/app.js): reduce parameters in parseAndTranslate Refactor (public/src/app.js): reduce parameters in parseAndTranslate Jan 24, 2026
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.

1 participant