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

APP-2437: update OrgDetails proto for ops dashboard improvements #599

Merged
merged 2 commits into from
Dec 13, 2024

Conversation

ohEmily
Copy link
Member

@ohEmily ohEmily commented Dec 4, 2024

APP-2437

Included public_namespace and org name as well to the new search message type because I can see us implementing these in time. Only doing search on CRM ID at this stage to keep the changes tightly scoped.

@github-actions github-actions bot added the safe to test committer is a member of this org label Dec 4, 2024
@ohEmily ohEmily added the ready-for-protos add this when you want protos to compile on every commit label Dec 4, 2024
Comment on lines +1367 to +1382
message SearchOrganizationsRequest {
optional string org_id = 1;
optional string org_name = 2;
optional string cid = 3;
optional string public_namespace = 4;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

[self] didn't use oneof per our eng guidelines

Avoid oneof except for enums, since non-scalar oneofs are hard to work with in most protoc generations

@ohEmily ohEmily removed the ready-for-protos add this when you want protos to compile on every commit label Dec 9, 2024
@ohEmily
Copy link
Member Author

ohEmily commented Dec 9, 2024

Apparently I need to remove the "ready for protos" label to stop these guys from getting stuck:
Screenshot 2024-12-09 at 3 00 43 PM

just removed the label, hopefully that helps

@ohEmily
Copy link
Member Author

ohEmily commented Dec 9, 2024

Abe pointed out I probably picked up a deprecation and got unlucky on timing, this guy: #595
Before I add //nolint:staticcheck I'll do one retry because it'll make my scope tighter if I can get it working without making further edits.

@ohEmily ohEmily added the ready-for-protos add this when you want protos to compile on every commit label Dec 9, 2024
@ohEmily ohEmily removed the ready-for-protos add this when you want protos to compile on every commit label Dec 9, 2024
@ohEmily ohEmily added ready-for-protos add this when you want protos to compile on every commit and removed protos-compiled labels Dec 12, 2024
@ohEmily ohEmily added ready-for-protos add this when you want protos to compile on every commit and removed ready-for-protos add this when you want protos to compile on every commit protos-compiled labels Dec 12, 2024
…earch features

APP-2437: Add SearchOrganizations gRPC
@ohEmily
Copy link
Member Author

ohEmily commented Dec 12, 2024

GitHub labeling integration is having a bad day. Doing some creative rebasing and force pushing to see if I can fix it.

@ohEmily ohEmily removed the ready-for-protos add this when you want protos to compile on every commit label Dec 13, 2024
Copy link

@JosephBorodach JosephBorodach left a comment

Choose a reason for hiding this comment

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

:shipit:

[I'm continuing to read/ramp up on how we use grcp in golang but the PR LGTM]

@ohEmily ohEmily merged commit a97363b into main Dec 13, 2024
3 checks passed
@ohEmily ohEmily deleted the APP-2437 branch December 13, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protos-compiled safe to test committer is a member of this org
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants