search

canonical / charm-relation-interfaces

https://canonical.github.io/charm-relation-interfaces/
Apache License 2.0
16 stars 48 forks source link

Add smtp relation interface #126

Closed arturo-seijas closed 11 months ago

arturo-seijas commented 11 months ago

Add SMTP interface definition, as implemented by the SMTP integrator charm. A sample charm consuming this interface can be found here. This relation interface defines how the SMTP details are shared with the required charm. Note that it supports both Juju Secrets and no secrets for legacy charms.

The spec defining this interface can be found here

arturo-seijas commented 11 months ago

@PietroPasotti @simskij @gruyaume , can you please review?

gruyaume commented 11 months ago

@PietroPasotti @simskij @gruyaume , can you please review?

Please add a description to the PR. I.e. What am I reviewing here and why?

benhoyt commented 11 months ago

@arturo-seijas I'm going to be a grumpy old man for a second and double down on what @gruyaume said: you're requesting our review time (maybe 20-30 minutes each), so if you could please put in a decent amount of effort on the PR description to set context, that would be much appreciated. Simply saying "Add SMTP interface definition" doesn't add anything significant over the PR title. It may only need to be two or three sentences. Something like:

IS DevOps has developed the xyz charm that provides an SMTP server. This relation interface is useful for charms connecting to such servers, providing host/port/auth details for the requiring charm. For example, see our recent charms abc and def for example consumers. Note that the relation interface being proposed supports both Juju Secrets (preferred) and no Secrets (as we still have some legacy charms) -- see README.md for more details.

All this might "go without saying" from your point of view, but for reviewers being pulled in, context and links like that are very helpful.

Also worth considering when tagging people for review is to say why you're asking them. It's somewhat obvious why you would want to tag Charm Tech (@canonical/charm-tech), but perhaps "@PietroPasotti could you please review the Scenario tests specifically? And @gruyaume we'd love your take as I know the Telco team has used SMTP in the past." (or whatever reason)