search

praekeltfoundation / vumi

Messaging engine for the delivery of SMS, Star Menu and chat messages to diverse audiences in emerging markets and beyond.
BSD 3-Clause "New" or "Revised" License
420 stars 131 forks source link

Rename status event level names #992

Closed justinvdm closed 8 years ago

justinvdm commented 8 years ago

At the moment we use good, minor and major. The plan is to change these to ok, degraded and down, since those are better descriptions for what the status events mean.

justinvdm commented 8 years ago

Ready for review.

jerith commented 8 years ago

I'd be happier if these were named constants, so it's harder to accidentally misspell them or something.

hodgestar commented 8 years ago

@jerith Doesn't message validation already check that?

jerith commented 8 years ago

Ah, I didn't see that. Thanks. :-)

I still think it makes more sense to use something more "enumeration-like" than arbitrary strings given that there is a fixed set of valid values, but I'm also okay with strings+validation.

justinvdm commented 8 years ago

@hodgestar Hmm, forgot that we do validation, good point.

Changed to using class-level variables, but happy to revert the change.

hodgestar commented 8 years ago

I'm okay with keeping the constants. They're sometimes a little annoying in Python because one ends up importing weird modules and classes just to access one string, but in Vumi itself that's not really an issue.

hodgestar commented 8 years ago

And we have validation too, so woot.

hodgestar commented 8 years ago

Other than the final tiny comment about a missed opportunity to used TransportStatus.STATUS_X, looks good to me.

justinvdm commented 8 years ago

Decided to revert the class-level variables approach, agree with this comment.

Ready for re-review.

hodgestar commented 8 years ago

:+1: