Skip to content

chore: adding code owner for confirmation domain's code#23575

Merged
jpuri merged 4 commits intodevelopfrom
confirmation_code_owner
Mar 26, 2024
Merged

chore: adding code owner for confirmation domain's code#23575
jpuri merged 4 commits intodevelopfrom
confirmation_code_owner

Conversation

@jpuri
Copy link
Copy Markdown
Contributor

@jpuri jpuri commented Mar 19, 2024

Description

Adding code owner for confirmation domain's code

Related issues

Fixes: MetaMask/MetaMask-planning#2261

Manual testing steps

NA

Screenshots/Recordings

NA

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've clearly explained what problem this PR is solving and how it is solved.
  • I've linked related issues
  • I've included manual testing steps
  • I've included screenshots/recordings if applicable
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.
  • I’ve properly set the pull request status:
    • In case it's not yet "ready for review", I've set it to "draft".
    • In case it's "ready for review", I've changed it from "draft" to "non-draft".

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@jpuri jpuri added the team-confirmations-secure-ux-PR PRs from the confirmations team label Mar 19, 2024
@jpuri jpuri requested review from a team and kumavis as code owners March 19, 2024 09:01
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 19, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

cryptotavares
cryptotavares previously approved these changes Mar 20, 2024
View reviewed changes
matthewwalsh0
matthewwalsh0 reviewed Mar 22, 2024
View reviewed changes
.github/CODEOWNERS Outdated
.devcontainer/ @MetaMask/library-admins @HowardBraham @plasmacorral @brad-decker

# Confirmations UX team to own code for confirmations on UI.
ui/pages/confirmations @MetaMask/confirmations-ux
Copy link
Copy Markdown
Member

@matthewwalsh0 matthewwalsh0 Mar 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Can we include the @MetaMask/confirmations-system-team here also?

We maintain the generic ConfirmationPage as part of the approval framework, plus we're currently working on alerts and simulations that will have some entries within here.

Copy link
Copy Markdown
Contributor

@brad-decker brad-decker Mar 22, 2024

Choose a reason for hiding this comment

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

I can approve once this change is made

Copy link
Copy Markdown
Contributor Author

@jpuri jpuri Mar 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I made the update, but found this issue for both the teams: ux and system. We need to fix think before we merge:

Screenshot 2024-03-22 at 9 39 02 PM

Copy link
Copy Markdown
Contributor

@brad-decker brad-decker Mar 22, 2024

Choose a reason for hiding this comment

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

Resolved, @jpuri - you all have write access via other methods but the team needs write access in order to be listed as codeowners, which you now have.

Copy link
Copy Markdown
Contributor Author

@jpuri jpuri Mar 22, 2024

Choose a reason for hiding this comment

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

Thanks @brad-decker

@jpuri jpuri dismissed stale reviews from cryptotavares and pedronfigueiredo via 52d6357 March 22, 2024 16:07
@metamaskbot
Copy link
Copy Markdown
Collaborator

metamaskbot commented Mar 22, 2024

Builds ready [b8da1ad]
Page Load Metrics (809 ± 459 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint703691507034
domContentLoaded116737199
load572387809955459
domInteractive116736199
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@jpuri jpuri merged commit 54c76f2 into develop Mar 26, 2024
@jpuri jpuri deleted the confirmation_code_owner branch March 26, 2024 14:32
@github-actions github-actions bot locked and limited conversation to collaborators Mar 26, 2024
@metamaskbot metamaskbot added the release-11.15.0 Issue or pull request that will be included in release 11.15.0 label Mar 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@matthewwalsh0 matthewwalsh0 matthewwalsh0 left review comments

@Gudahtt Gudahtt Gudahtt approved these changes

@pedronfigueiredo pedronfigueiredo pedronfigueiredo approved these changes

@cryptotavares cryptotavares cryptotavares approved these changes

@kumavis kumavis Awaiting requested review from kumavis kumavis is a code owner

+1 more reviewer

@brad-decker brad-decker brad-decker approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release-11.15.0 Issue or pull request that will be included in release 11.15.0 team-confirmations-secure-ux-PR PRs from the confirmations team

Projects

PR review queue
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@jpuri @metamaskbot @Gudahtt @brad-decker @pedronfigueiredo @cryptotavares @matthewwalsh0