Skip to content

Conversation

@mrashutoshnigam
Copy link
Contributor

Feature/Code Quality: Replace string literals with string constants in Files App

This pull request addresses the improvement of code quality in the Files App by replacing string literals with string constants. This change enhances maintainability, reduces the likelihood of typos, and improves overall code readability.

Changes Made

  • Replaced string literals with corresponding string constants throughout the Files App codebase.

Ensured all affected strings are now referenced using constants instead of hard-coded values.

Testing Steps

  • Open Files App
  • Navigated through various sections of the app to ensure all strings are displayed correctly
  • Performed actions that trigger string-dependent functionality (e.g., error messages, tooltips)
  • Update language via settings and repeat step 2 and 3
  • Verified that no visual or functional regressions occurred due to the changes
  • Checked for any compile-time errors or warnings related to string usage

Additional Notes

  • This change does not alter any functionality but improves code maintainability.
  • All string constants are now centralized, making future updates and localization efforts easier.
  • Code reviewers should pay special attention to ensure no string literals were missed during this refactoring.

@Lamparter
Copy link
Contributor

Lamparter commented Feb 19, 2025

This was already discussed against in #16718

Our approach is to implement these kinds of changes only to files that are already being modified. This minimizes the need for additional code reviews and ensures all modifications are thoroughly tested.

@yaira2 yaira2 force-pushed the codeQuality/replaceStringsLiteralsWithContants branch from 4a71058 to e2dc938 Compare February 20, 2025 21:58
@yaira2
Copy link
Member

yaira2 commented Feb 20, 2025

@mrashutoshnigam thank you for your contribution. As @Lamparter pointed out, we typically avoid large PRs like this since they can be challenging to review without causing the browser to freeze. That said, this change aligns with our goals and is something we were gradually working toward. If you’d be willing to split it into smaller PRs, I’d be able to review them more effectively without GitHub freezing 🙂

@yaira2 yaira2 closed this Feb 20, 2025
@mrashutoshnigam
Copy link
Contributor Author

Hi @yaira2 and @Lamparter,
Thank you for the feedback. As suggested, I’m closing this PR. I will split the changes into smaller, more manageable PRs with just a few files each. This should help streamline reviews and avoid issues like browser freezes.
Thanks again for your guidance—I'll post the new PRs shortly.

@mrashutoshnigam mrashutoshnigam deleted the codeQuality/replaceStringsLiteralsWithContants branch February 21, 2025 06:29
@yaira2
Copy link
Member

yaira2 commented Feb 21, 2025

more manageable PRs with just a few files each

My suggestion is to limit each PR to around 30 files.

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.

3 participants