Gem attributes are a mix of snake case and camel case - all should be snake case

[cmarkle]

The following attributes, defined with the sl_attr method are in camel case (to presumably match the lower-level naming) but to fit the Ruby idioms and the project style guidelines they should be in snake case (i.e., xxx_yyy_zzz, not zzzYyyZzz). Here are the ones I found:

lib/softlayer/Account.rb:    sl_attr :companyName
lib/softlayer/Account.rb:    sl_attr :firstName
lib/softlayer/Account.rb:    sl_attr :lastName
lib/softlayer/Account.rb:    sl_attr :postalCode
lib/softlayer/Account.rb:    sl_attr :officePhone

lib/softlayer/NetworkComponent.rb:    sl_attr :maxSpeed

lib/softlayer/ProductItemCategory.rb:    sl_attr :categoryCode

lib/softlayer/Server.rb:    sl_attr :fullyQualifiedDomainName

lib/softlayer/Ticket.rb:    sl_attr :lastEditDate

lib/softlayer/VirtualServer.rb:    sl_attr :provisionDate
lib/softlayer/VirtualServer.rb:    sl_attr :activeTransaction
lib/softlayer/VirtualServer.rb:    sl_attr :blockDevices
lib/softlayer/VirtualServer.rb:    sl_attr :lastOperatingSystemReload

Also the VLAN_number attributes, although in snake case, probably should have "VLAN" in lower-case ala "vlan_number".

lib/softlayer/VLANFirewall.rb:    sl_attr :VLAN_number, 'vlanNumber'

I realize that this in effect changes the API but we should get consistent and do it soon...

[ju2wheels]

We should probably start a v4 tracker thread to mark what needs to be removed theres some other stuff in here that needs moving too like Account#find_vlan_with_number should probably get moved into a NetworkVlan class once thats created in order to fit with the organization of the other objects. And it doesnt really fit with the other find methods that allowed passing of object filters (it could have been a more generic/powerful find_vlan method) and all find methods could be normalized to add a result limit as well so they follow the API usage get object service, filter, limit results, and call service method.

[SLsthompson]

Just so you know where some of this came from. The goal of sl_attr is really just a simple mechanism to expose the underlying attributes easily (so you didn't have to use ['attribute']). The only time I used a different name for the attribute was when I didn't think the low-level attribute's name did a good job of describing what the attribute was all about. Then I did the snake case for the "improved" names. This is all very subjective which is why it is also inconsistent. Adding consistency would be OK with me.

In broader terms, the modeling throughout the framework is incomplete because I wanted it to be "demand driven". My goals when creating what exists of the modeling layer was not to model everything, but to do "enough" to make your lives easier :-). In some cases (like the VLAN case) I made a judgement call between modeling things "correctly" and getting the functionality incorporated. Please do expand the models as needed to simplify your scripts.

[SLsthompson]

I created a v4.0 milestone that you can assign issues to if you would like.

[ju2wheels]

I can push this change today and mark the old ones with a deprecation warning and reference the new ones if we are ok with starting this transition to fix it for the above attributes.

This would allow us to enable the change now and do the removal of the old ones marked as deprecated with the release of v4 (at the expense of looking ugly in the doc but it allows a smoother transition).

Should we do the same for the class name? VLANFirewall = VlanFirewall .

We can also enable this now by renaming the class and add a alias under the class definition for compat IIRC:

class VlanFirewall
end

VLANFirewall = VlanFirewall

[SLsthompson]

My personal preference is to leave Acronyms and abbreviations in all upper case. I prefer VLAN to Vlan, URL to Url etc... But that's probably just because I'm overly pedantic.

The code is there for you guys to use, not for me to write. You may change it if you feel strongly about it.

[ju2wheels]

Excerpt from the Ruby Style Guide this project is following:

• Use CamelCase for classes and modules. (Keep acronyms like HTTP, RFC, XML uppercase.)

So the classes should stay as is but the VLAN_number attrib should still probably be lowercase.

[ju2wheels]

OK so the PR for this causes duplicate getter methods but allows for transition except in the case of ProductConfigurationOption where its going to also cause duplicate data as well because of it being a struct.

[cmarkle]

Made some comments in the PR. I like the transition idea.

[ju2wheels]

Im adding the xx_at but keeping them closely named (ie last_edited_at) that way its easier to tell which hash key it goes to. The at will help distinguish them from being confused for possible booleans in the case of created/modified.

[ju2wheels]

I think we are good now, all case cleanups and missing rdoc documentation are in and a few bug fixes with masks as well.

All commits that need to have something removed for v4 have been tagged v4-cleanup-required in the commit message, additionally for tracking purposes this is the deprecation/change list:

Class	Type	Deprecated Name	New Name
Account	Attribute	companyName	company_name
Account	Attribute	firstName	first_name
Account	Attribute	lastName	last_name
Account	Attribute	postalCode	postal_code
Account	Attribute	officePhone	office_phone
Account	Method	find_VLAN_with_number	find_vlan_with_number
BareMetalServerOrder_Package	Attribute	provision_script_URI	provision_script_uri
BareMetalServerOrder	Attribute	provision_script_URI	provision_script_uri
NetworkComponent	Attribute	maxSpeed	max_speed
NetworkMessageDelivery	Attribute	created	created_at
NetworkMessageDelivery	Attribute	modified	modified_at
NetworkStorageCredential	Attribute	created	created_at
NetworkStorageCredential	Attribute	modified	modified_at
NetworkStorageGroup	Attribute	created	created_at
NetworkStorageGroup	Attribute	modified	modified_at
NetworkStorage	Attribute	created	created_at
ProductItemCategory	Attribute	categoryCode	category_code
ProductConfigurationOption	Key	capacityRestrictionMaximum	capacity_restriction_maximum
ProductConfigurationOption	Key	capacityRestrictionMinimum	capacity_restriction_minimum
ProductConfigurationOption	Key	capacityRestrictionType	capacity_restriction_type
ProductConfigurationOption	Key	hourlyRecurringFee	hourly_recurring_fee
ProductConfigurationOption	Key	laborFee	labor_fee
ProductConfigurationOption	Key	oneTimeFee	one_time_fee
ProductConfigurationOption	Key	recurringFee	recurring_fee
ProductConfigurationOption	Key	requiredCoreCount	required_core_count
ProductConfigurationOption	Key	setupFee	setup_fee
Server	Attribute	fullyQualifiedDomainName	fqdn
SoftwarePassword	Attribute	created	created_at
SoftwarePassword	Attribute	modified	modified_at
Ticket	Attribute	last_modified	last_modified_at
UserCustomerExternalBinding	Attribute	created	created_at
UserCustomer	Attribute	created	created_at
UserCustomer	Attribute	modified	modified_at
VirtualDiskImage	Attribute	created	created_at
VirtualDiskImage	Attribute	modified	modified_at
VirtualServerOrder_Package	Attribute	provision_script_URI	provision_script_uri
VirtualServerOrder	Attribute	provision_script_URI	provision_script_uri
VirtualServer	Attribute	provisionDate	provisioned_at
VirtualServer	Attribute	activeTransaction	active_transaction
VirtualServer	Attribute	blockDevices	block_devices
VirtualServer	Attribute	lastOperatingSystemReload	last_operating_system_reload
VLANFirewall	Attribute	VLAN_number	vlan_number
VLANFirewall	Method	primaryRouter	primary_router
VLANFirewall	Method	fullyQualifiedDomainName	fqdn

I also added the missing result_limit option to find methods where it made sense (in some of the methods the last service call was in a loop and therefore adding a result limit would not give the expect output).

[SLsthompson]

The pull request related to this has been pulled in, but I forgot to add the comment that marks this fixed.