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

[PM-20142] Split GlobalSettings in file in the same namespace #5641

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

elyosemite
Copy link

📔 Objective

Restructure the monolithic GlobalSettings.cs file into separate class-specific files, adhering to the Single Responsibility Principle (SRP) and Separation of Concerns (SoC), to achieve the following goals:

  • Improve Maintainability:
    • Enable instant navigation to specific configuration classes (no more searching through 500+ lines)
    • Isolate changes to avoid unintended side effects in unrelated classes
  • Optimize Development Workflow:
    • Reduce merge conflicts during parallel work on unrelated settings
    • Enable granular Git history for clearer change tracking
- GlobalSettings.cs (1 file, N classes)  
+ Core/Settings/  
+ ├── AmazonSettings.cs
+ ├── AppleIapSettings.cs
+ ├── BaseServiceUriSettings.cs
+ ├── BitPaySettings.cs
+ ├── DistributedCacheSettings.cs  
+ ├── DistributedIpRateLimitingSettings.cs  
+ ├── InstallationSettings.cs  
+ └── ... (1 file per class)  

📸 Screenshots

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

…cationHubPoolSettings, NotificationHubSettings and NotificationsSettings classes from GlobalSettings.cs to files in the same namespace
…Settings, SsoSettings, StripeSettings, SyslogSettings, WebPushSettings and YubicoSettings classes from GlobalSettings.cs to file in the same namespace
…external, internal uri and building directories.
@S-Kakar
Copy link

S-Kakar commented Apr 11, 2025

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PM-20142
Link: https://bitwarden.atlassian.net/browse/PM-20142

Details on our contribution process can be found here: https://contributing.bitwarden.com/contributing/pull-requests/community-pr-process.

@S-Kakar S-Kakar changed the title Split GlobalSettings in file in the same namespace [PM-20142] Split GlobalSettings in file in the same namespace Apr 11, 2025
@elyosemite
Copy link
Author

@S-Kakar thank you for reply. However I don't have access to you Jira board because I'm contributor from the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants