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

fix: Name of team plan #738

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

IchordeDionysos
Copy link

@IchordeDionysos IchordeDionysos commented Dec 23, 2024

I did leave the enum value for the Subscription as it was...
Not sure what the current status is here:

- Professional Yearly

I actually also wasn't sure what to do with those dnsimple-professional plan ids.

@sbastn sbastn requested review from sbastn and san983 December 23, 2024 16:07
@sbastn sbastn added the documentation Updates on the documentation and support content. label Dec 23, 2024
@sbastn
Copy link
Member

sbastn commented Dec 23, 2024

Thank you, @IchordeDionysos, for submitting these changes. Keeping the Enum values as they are makes sense, especially since we still have customers subscribed to the Professional plan (now considered legacy).

@san983, everything looks good on my end. I do have a question about the fixtures—are they purely for illustrative purposes, or do we have tests or code that depend on the specific values defined in them?

Copy link
Member

@sbastn sbastn left a comment

Choose a reason for hiding this comment

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

Approving, pending @san983's review

@san983
Copy link
Member

san983 commented Dec 23, 2024

@san983, everything looks good on my end. I do have a question about the fixtures—are they purely for illustrative purposes, or do we have tests or code that depend on the specific values defined in them?

LGTM too. AFAIS this is just illustrative. But you'd have to double check with anyone from the app team.

@sbastn sbastn requested a review from jacegu December 24, 2024 08:50
@sbastn
Copy link
Member

sbastn commented Dec 24, 2024

@jacegu, I tagged you for a review to make sure the changes to the fixtures are OK and won't cause any issues if there’s code or tests relying on them (which I don't think we have, but I'm not 100% sure). Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Updates on the documentation and support content.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants