search

ManageIQ / azure-armrest

Ruby interface for Azure using the new REST API
Apache License 2.0
15 stars 36 forks source link

Handle possibility of colons in keys for model generation. #358

Closed robbmanes closed 6 years ago

robbmanes commented 6 years ago

Similarly to commit 845d11a (pr 329) it is possible for a colon to be present in keys of a BaseModel. This patch allows for colons to be converted to underscores like we do for periods and dollar signs.

Addresses https://bugzilla.redhat.com/show_bug.cgi?id=1548153

djberg96 commented 6 years ago

I'm wondering if we should just change it to [\W] at this point, i.e. anything that's not a letter or number.

robbmanes commented 6 years ago

Valid point - rebased commit to match \W for any/all special characters. If you think I should add a spec for each one let me know and I'd be happy to do so.

miq-bot commented 6 years ago

Checked commit https://github.com/robbmanes/azure-armrest/commit/50d3a91c705e014256f49ed799b98aa26f881f48 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 2 files checked, 0 offenses detected Everything looks fine. :cookie:

djberg96 commented 6 years ago

@robbmanes There are some existing specs for testing stuff. They seem to pass, but if you wouldn't mind adding one, that would be great. You can pretty much copy this:

https://github.com/ManageIQ/azure-armrest/blob/master/spec/models/base_model_spec.rb#L217-L222

robbmanes commented 6 years ago

Sorry, don't think I was very clear; I did in the rebase for 50d3a91 - I meant if you wanted more symbols than the ones I was testing against (added pound sign and colon) like parenthesis or anything else other than the ones I already added.

djberg96 commented 6 years ago

@robbmanes oops, nah, that should be fine.

djberg96 commented 6 years ago

👍