delfick / bespin

Opinionated wrapper around boto that reads yaml
MIT License
6 stars 8 forks source link

Add 'count' formatter #68

Closed atward closed 7 years ago

atward commented 7 years ago

Returns array length of CommaDelimitedList.

Was originally going to return {var:count} == 0 for var: "" (after strip), but decided to match CloudFormation as close as possible. Passing empty string to CloudFormation CommaDelimitedList results in [''] such that !Select [ 0, !Ref param ] returns ''.

Open to discussing name. Originally thought of 'length' but {var:length} to me means string length. Any other ideas seemed long or clunky.

delfick commented 7 years ago

I'm confused why the strip is needed?

Wouldn't it just be return obj.count(",") + 1 ?

Also, we should probably support when var points at a list instead of a string

On Thu, May 11, 2017, 11:07 AM Adam Ward notifications@github.com wrote:

@atward https://github.com/atward requested your review on: delfick/bespin#68 https://github.com/delfick/bespin/pull/68 Add 'count' formatter.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/delfick/bespin/pull/68#event-1077435092, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGq9SiWrfhGanizOqyDnyKBNcl3KVE0ks5r4l9UgaJpZM4NXZju .

atward commented 7 years ago

In hindsight it's not. I was originally implementing CommaDelimitedList and it was part of that.

delfick commented 7 years ago

Lgtm

On Thu, May 11, 2017, 11:32 AM Adam Ward notifications@github.com wrote:

In hindsight it's not. I was originally implementing CommaDelimitedList and it was part of that.

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/delfick/bespin/pull/68#issuecomment-300657559, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGq9dc8J1rSb7uWtGIq-1O28THVL29zks5r4mUpgaJpZM4NXZju .