Azure / azure-sdk-for-go

This repository is for active development of the Azure SDK for Go. For consumers of the SDK we recommend visiting our public developer docs at:
https://docs.microsoft.com/azure/developer/go/
MIT License
1.59k stars 821 forks source link

Review for renaming of stuttering collision of GO track2 SDK #16854

Closed JeffreyRichter closed 2 years ago

JeffreyRichter commented 2 years ago

I think as part of this exercise we’ll need to distill what exactly that guideline is. We don’t have one in place as originally we weren’t planning on removing stuttering.

I need to look at these in more detail, but my knee-jerk is that we shouldn’t be renaming the ones where the names had stuttering removed. i.e. ‘DataBoxJobDetails’ => ‘JobDetails’ should remain ‘JobDetails’ so that its fully qualified name is ‘databox.JobDetails’ (this matches what the swagger was conveying). We should instead rename the thing it collides with, i.e. ‘JobDetails’ is renamed to something slightly different.

-Joel

Subject: RE: Review for renaming of stuttering collision of GO track2 SDK

It would be great if we can base our decisions on guidelines.

FWIW I’m not a big fan of shuffling AaBb into BbForAa. Personally I’d rather stutter than name it like that. Ex: from: 'DataBoxJobDetails' to: 'JobDetailsForDataBox'

_rick

Subject: Review for renaming of stuttering collision of GO track2 SDK

Hi all,

As stuttering removing has been introduced into the latest autorest.go to make GO track2 API more idiomatic, we have encountered some collision for some existed models or clients. To let the renaming more reliable, we’d like to have a quick review through the mail. I’ve listed all such renaming in current preview GO SDKs with directives and the collision type in the following table. If any suggestion, please feel free to reply directly. Thanks.

rp directive type cdn directive:

Regards, Chenjie Shi

jhendrixMSFT commented 2 years ago

The guiding principle should be that the type that was renamed to not stutter should not further be renamed to avoid a name collision.

Using DataBoxEdge as the example, there are types "DataBoxEdgeSku" and "Sku". The former type is renamed to "Sku" to avoid stuttering which collides with the preexisting "Sku" type. To avoid the collision, the preexisting "Sku" type should be appropriately renamed (e.g. maybe "SkuInfo" or "SkuData"). So the resulting codegen would look like this.

package databoxedge

// SKU was renamed from DataBoxEdgeSku to not stutter.
type SKU struct { /*...*/ }

// SKUInfo was renamed via an Autorest directive to avoid colliding with SKU
type SKUInfo struct { /*...*/ }
tadelesh commented 2 years ago

Update for the renaming:

cdn directive:

databox directive:

databoxedge directive:

guestconfiguration directive:

labservices directive:

redis directive:

security directive:

signalr directive:

videoanalyzer directive:

webpubsub directive:

web (appservice) directive:

jhendrixMSFT commented 2 years ago

This has been resolved.