Support Vulnerability Alerts and Automated Security Fixes #286
Conversation
|
@travi thoughts? |
|
hey @aaricpittman 👋. sorry for the delay getting this reviewed. thanks a ton for the contribution. i do think this is valuable and want to get this merged. i just haven't had a chance yet to sit down and exercise it properly and give it a good review. based on my quick overview, this looks like a great start. one thing that comes to mind is that i'd like to see more separation in the scenarios at the integration test level. i havent looked close enough at the current state of things to suggest a detailed approach yet, but i would at least like to see a separate scenario for a project that does not define these at all and one that does, hopefully in a way that changing in different directions can be exercised. i will try to give this a closer look soon. |
|
also, we've been working through a major probot upgrade. you can see the current step here: #287 i may want to hold off actually merging until some of that settles a little more, but assuming i find the time, we can keep iterating here until then. |
|
@aaricpittman i'd like to try to get this moving forward. could you rebase your branch so that the upgrade to probot v10, that happened since you opened this PR, is included? the other piece that i would like to see included before looking to merge this would be the expansion of the integration tests that i mentioned above so that the various scenarios that are possible with this are exercised. i'm happy to help you through any details around that process that might be less than smooth, but i want to make sure that merging this new behavior would enable those that want to manage this detail, but also wouldn't break existing configs or those that arent interested in using this feature. |
|
@travi sorry, i just saw this. i'll get started on those changes. |
|
@travi i've updated the pr. is this what you had in mind? |
|
@aaricpittman apologies for the delay. this looks good to me, but we need to account for some changes that had to be applied to the tests for another probot upgrade. if you could rebase your branch against master to get the build passing again, i'm comfortable getting this merged in |
|
@travi i've updated the tests. |
|
looks good to me. thanks a lot for the contribution and for your patience with getting this integration. i really appreciate the help |
Fixes #230
View rendered README.md