Skip to content

Update subscription.go#10

Open
casalsgh wants to merge 2 commits intomainfrom
subscription-refactor
Open

Update subscription.go#10
casalsgh wants to merge 2 commits intomainfrom
subscription-refactor

Conversation

@casalsgh
Copy link
Copy Markdown

Removed unnecessary comments and added line comments where necessary.
Adjusted the indentation and formatting to enhance readability.
Consolidated the constant declarations for SubscriptionStatus to improve code organization.
Reordered the fields in the Subscription struct to group related fields together.
Renamed the IsUsageBased field in the Pricing struct to follow the convention of using "Is" as a prefix for boolean fields.
Moved the PricingRecurrence type declaration and constants to a more appropriate location within the file.
Retained the original linting rules and naming conventions while making the code more concise and readable.

Comment thread data/subscription.go Outdated
Comment thread data/subscription.go
SubscriptionStatusCancelled SubscriptionStatus = "cancelled" //nolint:misspell //cancelled is how we spell it

// SubscriptionStatusCancelling is when the subscription is being canceled
SubscriptionStatusCancelling SubscriptionStatus = "cancelling" //nolint:misspell //cancelled is how we spell it
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we might need to suppress this rule or it might need to remain a commend like this to keep linter from being unhappy about this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think I kept it, but still not able to pass the lint

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread data/subscription.go
SubscriptionStatusTrialing SubscriptionStatus = "trialing"
SubscriptionStatusActive SubscriptionStatus = "active"
SubscriptionStatusCancelled SubscriptionStatus = "cancelled"
SubscriptionStatusCancelling SubscriptionStatus = "cancelling"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

gone from here

@geekgonecrazy
Copy link
Copy Markdown
Contributor

geekgonecrazy commented Jun 27, 2023

Coming back at this with fresh eyes.

In general with this being public repo and there being the possibility of others consuming it and need to understand the various states.. I don't know that removing these comments is actually making anything better. I feel while its a bit easier to read.. in this case knowing what these states do its worth it having more comments

Also if you look at go docs you can see a bit on how its useful: https://pkg.go.dev/github.com/RocketChat/marketplace-go@v1.2.1/events

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