Skip to content

feat: Azure more new new resource manager SDK#5286

Closed
yevgenypats wants to merge 5 commits intomainfrom
feat/azure_arm
Closed

feat: Azure more new new resource manager SDK#5286
yevgenypats wants to merge 5 commits intomainfrom
feat/azure_arm

Conversation

@yevgenypats
Copy link
Copy Markdown
Contributor

This is a draft PR that helps me review #4768. I'll either apply fixes on top #4768 or will use this one directly if it will be much better.

Copy link
Copy Markdown
Contributor

@candiduslynx candiduslynx left a comment

Choose a reason for hiding this comment

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

Not sure about http mocks at all

if err != nil {
return services, err
}
services.ArmkeyvaultVaults = ArmkeyvaultVaults
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.

a good idea to init in-place.
I'd go even further and do it like this:

var err error
var services Services

services.ClientA, err = initClientA(...)
if err != nil {
    return services, err
}
...

Maybe I'll add it to my PR codegen as well

return nil, err
}
emptyStr := ""
item.NextLink = &emptyStr
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.

Suggested change
item.NextLink = &emptyStr
item.NextLink = nil

OR

Suggested change
item.NextLink = &emptyStr
item.NextLink = to.Ptr("")

emptyStr := ""
item.NextLink = &emptyStr
mux := httprouter.New()
mux.GET("/*filepath", func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
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.

I personally don't see a lot of difference between the generated interface + mock on it vs transport-based mock tests, but prefer the interface+mocks better.



type Client struct {
subscriptions []string
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.

this info is already present in services as keys

@yevgenypats yevgenypats closed this Dec 4, 2022
yevgenypats added a commit that referenced this pull request Dec 11, 2022
This PR comes instead of this
#5286 as old pr got messed
up.

This generates the autogeneration code and tries to generate the whole
huge Azure SDK completely.

Co-authored-by: Erez Rokah <erezrokah@users.noreply.github.com>
@hermanschaaf hermanschaaf deleted the feat/azure_arm branch November 21, 2023 08:48
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.

3 participants