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.

Sign up for GitHub

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

Jump to bottom

permissions-center: make PermissionSyncCodeHostState's status typed. #49834

Merged
merged 1 commit into from
Mar 22, 2023
Merged

permissions-center: make PermissionSyncCodeHostState's status typed. #49834

merged 1 commit into from
Mar 22, 2023

Conversation

sashaostrikov
Copy link
Contributor

This commit makes status typed instead of a raw string which will be further used in API and UI layers as well for increased stability and removal of possibility of unexpected string results being passed over API.

API/UI changes will be made in a follow-up commit.

Test plan:
Unit tests updated.

@sashaostrikov
permissions-center: make PermissionSyncCodeHostState's status typed.
ceb54b4 
This commit makes status typed instead of a raw string which will be further used in API and UI layers as well for increased stability and removal of possibility of unexpected string results being passed over API.

API/UI changes will be made in a follow-up commit.

Test plan:
Unit tests updated.
@sashaostrikov sashaostrikov added the permissions-center Tracking issue for all-things permissions center. label Mar 22, 2023
@sashaostrikov sashaostrikov requested a review from a team March 22, 2023 12:20
@sashaostrikov sashaostrikov self-assigned this Mar 22, 2023
@cla-bot cla-bot bot added the cla-signed label Mar 22, 2023
@sourcegraph-bot
Copy link
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 31d6d07...ceb54b4.

Notify File(s)
@indradhanush enterprise/cmd/repo-updater/internal/authz/integration_test.go
enterprise/cmd/repo-updater/internal/authz/perms_syncer_test.go
enterprise/cmd/repo-updater/internal/authz/perms_syncer_worker.go
enterprise/cmd/repo-updater/internal/authz/perms_syncer_worker_test.go
@unknwon enterprise/cmd/frontend/internal/authz/resolvers/permissions_sync_jobs.go
enterprise/cmd/repo-updater/internal/authz/integration_test.go
enterprise/cmd/repo-updater/internal/authz/perms_syncer_test.go
enterprise/cmd/repo-updater/internal/authz/perms_syncer_worker.go
enterprise/cmd/repo-updater/internal/authz/perms_syncer_worker_test.go

kopancek
kopancek approved these changes Mar 22, 2023
View reviewed changes
Copy link
Contributor

@kopancek kopancek left a comment

Choose a reason for hiding this comment

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

Was thinking about suggesting the same change. You're reading my mind dude 👍

@sashaostrikov sashaostrikov merged commit ccf9b20 into main Mar 22, 2023
@sashaostrikov sashaostrikov deleted the ao/pc/make-provider-state-typed branch March 22, 2023 13:03
@sourcegraph-bot-devx sourcegraph-bot-devx mentioned this pull request Mar 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pjlast pjlast pjlast approved these changes

@kopancek kopancek kopancek approved these changes

Assignees

@sashaostrikov sashaostrikov

Labels
cla-signed permissions-center Tracking issue for all-things permissions center.
Projects
No open projects
IAM Team
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sashaostrikov @sourcegraph-bot @pjlast @kopancek