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 domain_data to Organizations Create and Update methods to replace domains #291

Conversation

mthadley
Copy link
Contributor

Description

Adds a new domain_data parameter to be passed to Organization create and update calls, instead of the domains parameter which will be soon deprecated.

While I was in here I also cleaned up some noise and warnings when running the specs, and also unified the way we were calling warn into a shared WorkOS::Deprecation module.

Documentation

Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.

[ ] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

Copy link

linear bot commented Apr 30, 2024

@mthadley mthadley requested review from a team and marktran and removed request for a team April 30, 2024 01:42
These assertions don't really care about the internal behavior of
`WorkOS::Deprecation`.
@mthadley mthadley requested review from cmatheson and alisherry April 30, 2024 22:41
# @option domain_data [String] domain The domain that belongs to the organization.
# @option domain_data [String] state The state of the domain. "verified" or "pending"
# @param [Array<String>] domains List of domains that belong to the organization.
# Deprecated: Use domain_data instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

is this deprecation notice supposed to be attached to the old domains param? (i think we lost that one?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it should be associated with domains:

      # @param [Array<String>] domains List of domains that belong to the organization.
      #   Deprecated: Use domain_data instead.

Maybe it looks a bit weird since it is on the next line, indented, as a separate "paragraph". Is that the right way to document it in Ruby?

Copy link
Contributor

Choose a reason for hiding this comment

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

ohhh, i don't know how i wasn't seeing that earlier. yeah this looks right to me 👍

@mthadley mthadley merged commit 2766178 into main Apr 30, 2024
4 checks passed
@mthadley mthadley deleted the feature/auth-2969-ruby-add-domains_data-to-sdks-and-deprecate-domains branch April 30, 2024 23:45
@mthadley mthadley mentioned this pull request May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants