Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split on commas in CC environment variable for chapel_commits emails #12

Merged
merged 4 commits into from Jan 17, 2019

Conversation

bradcray
Copy link
Member

We're currently observing that a comma-separated list of email
addresses in the GITHUB_COMMIT_EMAILER_RECIPIENT_CC environment
variable is only sending to the first address rather than all of them,
in spite of the fact that the message header looks as though it
contains all of them using a reasonable format.

This applies a .split(",") to the variable before passing it to the
envelopes initializer to see whether splitting it into a list of
addresses works better.

Brad Chamberlain added 4 commits January 17, 2019 13:05
…mails

We're currently observing that a comma-separated list of email
addresses in the GITHUB_COMMIT_EMAILER_RECIPIENT_CC environment
variable is only sending to the first address rather than all of them,
in spite of the fact that the message header looks as though it
contains all of them using a reasonable format.

This applies a .split(",") to the variable before passing it to the
envelopes initializer to see whether splitting it into a list of
addresses works better.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 98.299% when pulling 73f0adb on bradcray:split-commas into 97a8fba on chapel-lang:master.

@bradcray
Copy link
Member Author

@benharsh , @jhoole : This is fairly superstitious, but may be worth a shot (if you agree). Jenna's setting of the environment variable to include all of our email addresses, comma-separated (as an O365 workaround until we can wrestle with it further) only seems to be sending to the first email on the list and not the others in spite of the fact that the message's header seems to be formatted properly and includes all the addresses. Here, I'm wondering whether calling split() on the comma-separated string to turn it into a list of strings, one per email address, will work better. The documentation for envelopes that Ben found for me over lunch suggested that the cc_addr field could be a list of addresses: http://tomekwojcik.github.io/envelopes/api/envelope.html

I have no idea how to test this short of dropping it in and seeing what blows up now that I've gotten it past Travis.

I'm also not at all insistent that we try this, but trying to be sensitive to the grumbling about how long these mails have been out.

@jhoole
Copy link

jhoole commented Jan 17, 2019

I think the worst that can happen is that it doesn't work which it's already not doing.

@bradcray bradcray merged commit b3a3679 into chapel-lang:master Jan 17, 2019
@bradcray bradcray deleted the split-commas branch January 17, 2019 22:38
@bradcray
Copy link
Member Author

@jhoole: It looks like nothing is still what's happening. Did you receive more mails after this was merged? Anything look weird/different about the Cc: line or the logs?

@jhoole
Copy link

jhoole commented Jan 18, 2019

I'm still getting mails. There's nothing weird looking I can see in the Cc: line, the source view, or the Heroku logs (though the Heroku logs don't show the Cc: line).

@jhoole
Copy link

jhoole commented Jan 18, 2019

Since I'm getting the mails, I'm going to see if I can do something weird in my inbox (forwarding the mails to chapel_commits to chapel_commits to see if it'll accept it once the mail is in outlook already).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants