-
Notifications
You must be signed in to change notification settings - Fork 750
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
Support "code owner" alerts #4075
Comments
Do we have a mail server that can be used to send email? If so, I can suggest an easy way to avoid modifying the GitHub workflow structure or writing separate workflows for every notification requirement. Instead, we can achieve this using a combination of simple scripting and a GitHub Action. Proposed WorkflowAssumptions Implementation Details We will introduce a
We create a simple shell script (
Triggering the Script with GitHub Actions
|
Really good stuff @oronno . Thanks. We can use SMTP_SERVER=smtp.gmail.com I think we can use [email protected] as the sending account. I have the login info. Will put it on my list to test these scripts over the next week or two. |
@oronno - I've been playing around with this proposal, but ran into a few things.
So I think the script needs to loop through all the changes first, placing them into email buckets, building up the message body for each party affected by the PR. Then at the end, loop through those messages and send. As an aside, did you actually get the above code to work? I tried it verbatim and it didn't work for me. The secrets had to be in the workflow yaml. Still haven't gotten the
All the examples out there seem to indicate the git diff should be in the workflow, not the script. |
Hey @bretg Sorry, there was a mismatch - main branch vs master branch. If you change to this, it should work correctly. I did a test run. Check the branch Run the script as following: Instead of sending email, I just printed the email body. It suppose to work in the same way. |
And regarding your concept of using "email bucket" to send single email for multiple code path changes as defined in multiple entry in codepath-notification file, I have successfully modified the script to achieve this! The send_notification_on_change.sh script should now handle scenarios where there are multiple entries in the codepath-notification file correctly. To test it, I made a commit that touches multiple files. As expected, the script sends only one consolidated email for the changes. |
I struggled with this for a while and got it all working in Node except for the email part. That's totally not working and my laptop refuses to build the necessary npm packages. Will try it again someday. |
Got it working. PR for PBS-Java is prebid/prebid-server-java#3645 |
We had a request in the Prebid Slack channel to alert the maintainer when a PR has been opened that affects a bid adapter. This is a pretty reasonable request and not the first time it's come up.
The easiest way to do this would be to turn on the github "code owner" feature so updates to bid adapters and modules at least alert the team that originally wrote it. I played around with the 'code owner' feature and I think it's ok with 3 wrinkles:
So here's a possible workflow:
An alternate solution (maybe even the preferred solution) would be something similar to CODEOWNERS but that just alerted the maintainer email address with an FYI. An action like the one below could work, but we would need a file for everyone who wanted to do this and github doesn't allow directory structure under
workflows
. So that directory could get messy with that approach.I haven't found any options out there for a CODEOWNERS-like config file.
The text was updated successfully, but these errors were encountered: