-
Notifications
You must be signed in to change notification settings - Fork 188
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
chore: project resource migration to new sdk #1686
Conversation
options := &matlas.ListOptions{ | ||
PageNum: int(stateModel.PageNum.ValueInt64()), | ||
ItemsPerPage: int(stateModel.ItemsPerPage.ValueInt64()), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason to ignore pagination now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With new SDK, ListProjects
uses default values for pagination instead of the values from stateModel
. Fixed it so I don't change the behaviour. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the old SDK should've had defaults as well, reason to ask is I have no context where stateModel
was and if those values where meaningful
results := make([]*tfProjectDSModel, 0, len(projectsRes.Results)) | ||
for _, project := range projectsRes.Results { | ||
atlasTeams, atlasLimits, atlasProjectSettings, err := getProjectPropsFromAPI(ctx, conn, connV2, project.ID) | ||
for i, project := range projectsRes.Results { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it's better not to mix index and object in the loop, so in this case I recommend:
for i := range projectsRes.Results {
project := &projectsRes.Results[i]
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion on this, but I am curious about why we should not mix object and index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good overall, minor comments
|
Description
Migrate
project
resource to new SDKLink to any related issue(s): https://jira.mongodb.org/browse/CLOUDP-213827
Type of change:
Required Checklist:
Further comments