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

Improve acronym function names #250

Conversation

jplewa
Copy link
Member

@jplewa jplewa commented Jul 13, 2023

Task

Resolves: #221

Description

IAMBinding -> iamBinding
IamAuditConfig -> iamAuditConfig
OrganizationPolicy -> organizationPolicy

This one is a bit awkward, but it's not our fault they called it VMware instead of Vmware:
VMwareCluster -> vMwareCluster

@jplewa jplewa requested a review from a team as a code owner July 13, 2023 10:46
@jplewa jplewa self-assigned this Jul 13, 2023
@jplewa jplewa requested review from myhau and ddzikon and removed request for a team July 13, 2023 10:46
@jplewa jplewa changed the title fix acronym function names Improve acronym function names Jul 13, 2023
@jplewa jplewa force-pushed the 228-rename-providerresource-to-provider-nameprovider branch from 062cfee to 28b17f6 Compare July 14, 2023 16:40
@jplewa jplewa force-pushed the 221-dsl-ergonomics-weird-names-of-resources-that-start-with-an-acronym branch 2 times, most recently from 3f9e935 to 6087622 Compare July 14, 2023 16:44
@jplewa jplewa requested a review from myhau July 14, 2023 16:44
@myhau
Copy link
Member

myhau commented Jul 17, 2023

For the record, here is the list with "tricky" type/resource names for providers supported by pulumi-kotlin (kudos to @jplewa for generating it):

Open the list

AAAARecord
AADDataConnector
AATPDataConnector
ACIService
ACL
ADCCatalog
ADLSGen1FileDataSet
ADLSGen1FolderDataSet
ADLSGen2FileDataSet
ADLSGen2FileDataSetMapping
ADLSGen2FileSystemDataSet
ADLSGen2FileSystemDataSetMapping
ADLSGen2FolderDataSet
ADLSGen2FolderDataSetMapping
AFDCustomDomain
AFDEndpoint
AFDOrigin
AFDOriginGroup
AKSService
APIService
APIServiceList
APIServicePatch
ARecord
ASCDataConnector
AScript
CACertificate
CNameRecord
CRL
CSIDriver
CSIDriverList
CSIDriverPatch
CSINode
CSINodeList
CSINodePatch
CSIStorageCapacity
CSIStorageCapacityList
CSIStorageCapacityPatch
DBAuditInstance
DBCluster
DBClusterLakeVersion
DBClusterParameterGroup
DBInstance
DBParameterGroup
DBProxy
DBProxyEndpoint
DBProxyTargetGroup
DBSubnetGroup
DHCPOptions
DNSSEC
DRTAccess
EC2Fleet
EIP
FHIRDatastore
GCPolicy
HAVip
HAVipAttachment
HBaseCluster
IAMAuditConfig
IAMBinding
IAMCustomRole
IAMMember
IAMPolicy
IPAM
IPAMAllocation
IPAMPool
IPAMPoolCidr
IPAMResourceDiscovery
IPAMResourceDiscoveryAssociation
IPAMScope
IPAddress
IPAddressList
IPAddressPatch
IPGroup
IPGroupCIDR
IPSet
IPv6FirewallRule
IVPCEConfiguration
MCASDataConnector
MDATPDataConnector
MECRole
MHSMPrivateEndpointConnection
MLTransform
MSIXPackage
OIDCProvider
PTRRecord
SAMLProvider
SAPApplicationServerInstance
SAPCentralInstance
SAPDatabaseInstance
SAPVirtualInstance
SRVRecord
SSLCertificate
SSLPolicy
TIDataConnector
URLMap
VCenter
VMWareReplicationPolicy
VMwareCluster
VMwareCollector
VMwareNodePool
VNetPeering
VPC
VPCConnection
VPCDHCPOptionsAssociation
VPCEConfiguration
VPCEndpoint
VPCEndpointService
VPCEndpointServicePermissions
VPCPeeringConnection
VPNConnection
VPNConnectionRoute
VPNGateway
VPNTunnel
WCFRelay
WCFRelayAuthorizationRule

I'm wondering how we handle these:

  • IPv6FirewallRule
  • IPGroupCIDR
  • EC2Fleet

@jplewa
Copy link
Member Author

jplewa commented Jul 19, 2023

I'm wondering how we handle these:

  • IPv6FirewallRule
  • IPGroupCIDR
  • EC2Fleet

IPv6FirewallRule -> iPv6FirewallRule
IPGroupCIDR -> ipGroupCIDR
EC2Fleet -> eC2Fleet

The first two seem fine to me. iPv6FirewallRule is a bit awkward, but I'd say it's correct looking at the actual class name. Tbh, the actual class name is weird to me, since I'd rather call the class Ipv6FirewallRule (see: https://stackoverflow.com/questions/34529999/camel-case-name-for-class-with-ipv4). The only one I'm wondering about is EC2Fleet. Maybe the rule should be "make upper case letters lowercase until you encounter the final upper-case letter or a number"? So that we end up with ec2Fleet?

@myhau
Copy link
Member

myhau commented Jul 19, 2023

The first two seem fine to me. iPv6FirewallRule is a bit awkward, but I'd say it's correct looking at the actual class name.

I think it's weird, but there is probably nothing we could do about it now, so let's leave it as is.

Maybe the rule should be "make upper case letters lowercase until you encounter the final upper-case letter or a number"? So that we end up with ec2Fleet?

Yep, I think that might be better actually.

@jplewa jplewa force-pushed the 221-dsl-ergonomics-weird-names-of-resources-that-start-with-an-acronym branch from 1f06f98 to 8504482 Compare July 19, 2023 10:59
@jplewa jplewa requested a review from myhau July 19, 2023 10:59
@jplewa jplewa force-pushed the 221-dsl-ergonomics-weird-names-of-resources-that-start-with-an-acronym branch from 8504482 to b302703 Compare July 19, 2023 10:59
@jplewa jplewa force-pushed the 228-rename-providerresource-to-provider-nameprovider branch from 28b17f6 to 3d03e16 Compare July 19, 2023 10:59
Copy link
Member

@myhau myhau left a comment

Choose a reason for hiding this comment

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

LGTM. Left a couple of minor comments, but feel free to merge now.

@jplewa jplewa force-pushed the 228-rename-providerresource-to-provider-nameprovider branch from 3d03e16 to 8e97914 Compare July 21, 2023 10:36
@jplewa jplewa force-pushed the 221-dsl-ergonomics-weird-names-of-resources-that-start-with-an-acronym branch from b302703 to 529773a Compare July 21, 2023 10:36
@jplewa
Copy link
Member Author

jplewa commented Jul 21, 2023

LGTM. Left a couple of minor comments, but feel free to merge now.

Can't merge, because this is branched off of #241 ;) EDIT: That might be not necessary, not sure how many conflicts there would be. I might "unrebase" if I find some free time

@jplewa jplewa force-pushed the 221-dsl-ergonomics-weird-names-of-resources-that-start-with-an-acronym branch from 529773a to 2c92ad6 Compare July 21, 2023 10:56
Base automatically changed from 228-rename-providerresource-to-provider-nameprovider to main July 26, 2023 09:17
jplewa added 4 commits July 26, 2023 11:18
Signed-off-by: Julia Plewa <jplewa@virtuslab.com>
Signed-off-by: Julia Plewa <jplewa@virtuslab.com>
Signed-off-by: Julia Plewa <jplewa@virtuslab.com>
Signed-off-by: Julia Plewa <jplewa@virtuslab.com>
jplewa added 3 commits July 26, 2023 11:18
Signed-off-by: Julia Plewa <jplewa@virtuslab.com>
Signed-off-by: Julia Plewa <jplewa@virtuslab.com>
Signed-off-by: Julia Plewa <jplewa@virtuslab.com>
@jplewa jplewa force-pushed the 221-dsl-ergonomics-weird-names-of-resources-that-start-with-an-acronym branch 2 times, most recently from d003004 to a2991c9 Compare July 26, 2023 09:26
Signed-off-by: Julia Plewa <jplewa@virtuslab.com>
@jplewa jplewa force-pushed the 221-dsl-ergonomics-weird-names-of-resources-that-start-with-an-acronym branch from a2991c9 to 27c924b Compare July 26, 2023 09:35
@jplewa jplewa merged commit 47abb0a into main Jul 26, 2023
@jplewa jplewa linked an issue Jul 26, 2023 that may be closed by this pull request
@jplewa jplewa deleted the 221-dsl-ergonomics-weird-names-of-resources-that-start-with-an-acronym branch July 26, 2023 09:52
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.

DSL ergonomics: weird names of resources that start with an acronym
2 participants