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

Add console bulk delete #2641

Merged
merged 3 commits into from
Jan 22, 2025
Merged

Add console bulk delete #2641

merged 3 commits into from
Jan 22, 2025

Conversation

ptkach
Copy link
Collaborator

@ptkach ptkach commented Jan 14, 2025

This change is Reviewable

@ptkach ptkach requested a review from gbrodman January 14, 2025 22:09
@ptkach ptkach force-pushed the consoleBulkUpdate branch 2 times, most recently from f1ab9bf to 3911c7d Compare January 16, 2025 16:39
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

need to fix the PR message (Github does this when you have multiple commits as part of one PR)

also I think the screenshot test needs to be updated

overall this seems good to me, i definitely don't have any better ideas of ways to do this and it looks good on alpha

Reviewed 10 of 12 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ptkach)


console-webapp/src/app/domains/domainList.component.ts line 292 at r2 (raw file):

          this._snackBar.open(err.error || err.message),
      });
  }

nit: newline

@ptkach ptkach force-pushed the consoleBulkUpdate branch from 3911c7d to 08bf589 Compare January 16, 2025 19:06
@ptkach ptkach changed the title Console bulk delete Add console bulk delete Jan 16, 2025
Copy link
Collaborator Author

@ptkach ptkach left a comment

Choose a reason for hiding this comment

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

need to fix the PR message
Done

also I think the screenshot test needs to be updated
I have it fixed by removing a spell check from the input, it's started causing flakiness our of the blue.

Reviewable status: 11 of 13 files reviewed, 2 unresolved discussions (waiting on @gbrodman and @Github-advanced-security[bot])

@ptkach ptkach force-pushed the consoleBulkUpdate branch from 08bf589 to d19f2b1 Compare January 16, 2025 20:08
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r3, 3 of 3 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ptkach)

Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ptkach)

@ptkach ptkach force-pushed the consoleBulkUpdate branch from d19f2b1 to 93ee38d Compare January 17, 2025 15:32
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ptkach)

@ptkach ptkach force-pushed the consoleBulkUpdate branch from 93ee38d to 14f82f6 Compare January 17, 2025 18:39
Copy link
Collaborator Author

@ptkach ptkach left a comment

Choose a reason for hiding this comment

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

Reviewable status: 13 of 18 files reviewed, 1 unresolved discussion (waiting on @gbrodman and @Github-advanced-security[bot])

console-webapp/src/app/domains/domainList.service.ts Fixed Show resolved Hide resolved
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Github-advanced-security[bot])

@ptkach ptkach force-pushed the consoleBulkUpdate branch from 14f82f6 to 2ad081f Compare January 17, 2025 20:14
@ptkach ptkach enabled auto-merge January 17, 2025 20:15
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Github-advanced-security[bot])

@ptkach ptkach force-pushed the consoleBulkUpdate branch 2 times, most recently from ee6efff to f63eb6c Compare January 21, 2025 19:47
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Github-advanced-security[bot])

@ptkach ptkach force-pushed the consoleBulkUpdate branch from f63eb6c to 0bf2066 Compare January 21, 2025 21:34
Copy link
Collaborator

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r9, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Github-advanced-security[bot])

@ptkach ptkach added this pull request to the merge queue Jan 22, 2025
Merged via the queue into google:master with commit e3c386a Jan 22, 2025
8 of 9 checks passed
@ptkach ptkach deleted the consoleBulkUpdate branch January 22, 2025 16:45
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.

2 participants