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

Update CODEOWNERS file #10

Merged
merged 3 commits into from
Aug 29, 2023
Merged

Update CODEOWNERS file #10

merged 3 commits into from
Aug 29, 2023

Conversation

matt-livefront
Copy link
Collaborator

@matt-livefront matt-livefront commented Aug 24, 2023

🚧 Type of change

  • 🎂 Other

📔 Objective

Updates the CODEOWNERS file to assign teams based on the expected project structure.

We're planning on using the following project structure. This may evolve and grow as we build out the project, but we'll try to group things based on the team that should be assigned to it.

- Bitwarden
- BitwardenActionExtension
- BitwardenAutoFillExtension
- BitwardenShareExtension
- BitwardenShared
    - Core
        - Auth
            - Models
            - Repositories
            - Services
        - Platform
            - Models
            - Repositories
            - Services
        - Tools
            - Models
            - Repositories
            - Services
        - Vault
            - Models
            - Repositories
            - Services
    - UI
        - Auth
            - SignUp
            - Login
        - Platform
            - Common (styles, extensions, reusable views)
            - Settings
        - Tools
            - Generator
            - Send
        - Vault
            - List
            - Detail

Open questions:

  • It's not clear to me what all the platform team should be assigned to. Is this everything that's specific to iOS and common/shared code? Would this include things similar to the following?
    • Styling (text, color, themes)
    • App storage, push notifications, general networking, app architecture etc
    • Localizations
    • Config files (e.g. Info.plist, entitlements)
  • Are there things that should be assigned to team-leads-eng or should every file ideally have an assigned code owner?

📋 Code changes

  • CODEOWNERS: Updated to assign code owners based on the project structure.

📸 Screenshots

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation 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

@vvolkgang
Copy link
Member

We have been discussing different approaches internally (having team folders closer to the root) but, not having any files yet makes the decision process harder. One thing we agree, based on the current structure it will be easy to pivot at a later stage.

I'm chasing feedback about the Platform ownership, what you described makes sense but I'm not sure about L10N for instance. Tools also own exporters.

Are there things that should be assigned to team-leads-eng or should every file ideally have an assigned code owner?

They're the default assignee when new changes don't fall under any defined ownership, that team is then responsible for defining who owns them. It was recently renamed to tech-leads, we need to update this file.

@matt-livefront
Copy link
Collaborator Author

@vvolkgang Updated based on feedback from our meeting today!

Copy link
Member

@vvolkgang vvolkgang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks for the update! 👍🏾

Copy link
Contributor

@nathan-livefront nathan-livefront left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@matt-livefront matt-livefront merged commit d59457e into main Aug 29, 2023
@matt-livefront matt-livefront deleted the matt/codeowners branch August 29, 2023 21:42
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.

4 participants