search

betagouv / betaGouvBot

Automated assistant for beta.gouv.fr administrative tasks on slack.
https://beta.gouv.fr
MIT License
4 stars 10 forks source link

Refactoring rules and mail formatting #80

Closed bonjourmauko closed 7 years ago

Morendil commented 7 years ago

We're green, and we have some time until the next date when we're sending notifications (june 15th). Let's merge and forge on with refactoring.

I would also like to improve our confidence that refactorings do not cause regressions when talking with external APIs such as SendGrid, OVH or Github, perhaps by providing endpoints for manual testing specifically that for instance will send out a test email or manipulate a mailing list set aside for testing purposes, etc. WDYT?

Morendil commented 7 years ago

Hmm, this was not actually mergeable - Mailer is still expecting the old-style rules, but as mailer_spec.rb feeds it just that, we're still green even though (I strongly suspect) production is broken. #83 fixes this but it worries me that we can get in that situation.

bonjourmauko commented 7 years ago

@Morendil Yep, that's why I added a comment "this should fail, but it doesn't".

Morendil commented 7 years ago

Haha, missed it - or rather I saw it but underestimated its significance.

bonjourmauko commented 7 years ago

Yeah, it is more common in a dynamic language than in a compiled one. That's when code review's importance comes in 😄 .