Charcoal-SE / SmokeDetector

Headless chatbot that detects spam and posts links to it in chatrooms for quick deletion.
https://metasmoke.erwaysoftware.com
Apache License 2.0
474 stars 182 forks source link

Prevent TravisCI build on blacklist requests/watchlist requests #828

Closed AWegnerGitHub closed 6 years ago

AWegnerGitHub commented 7 years ago

Is there a way to prevent Travis builds on blacklist requests/watchlists? There are no specific unit tests tied to the individual blacklist items.

AWegnerGitHub commented 7 years ago

Would there be any unintended side effect of putting [ci skip] on these commits?

https://docs.travis-ci.com/user/customizing-the-build/#Skipping-a-build

j-f1 commented 7 years ago

Not AFAIK. It should probably get added to the pull request title so that it shows up in the merge commit, too.

ArtOfCode- commented 7 years ago

Honestly, I'd quite like to keep CI, at least on the merge commits. Given that blacklist commits are auto generated, if the generator goes wrong, CI gives us a chance at catching it before it goes live. I can get behind skipping it on the branch commit, that'd save us a couple minutes, but for the merge to master I think we can trade two minutes for a sanity check.

AWegnerGitHub commented 7 years ago

What are you expecting to catch, @ArtOfCode-?

We don't have tests to test the new blacklist items.

ArtOfCode- commented 7 years ago

No, but we do test datahandling, which is the most likely candidate to break if a generator messes up a format or similar.

angussidney commented 7 years ago

Note that we do have some unit tests to prevent duplicate blacklist entries etc. however, I think Smokey intercepts these blacklists before they are commuted, so I think shouldn't be much of a problem.

Currently, Smokey requires the CI build to pass before it pulls the commit in. Will some changes be required for Metasmoke in order to allow no CI build at all?

ArtOfCode- commented 7 years ago

Nope, @angussidney - just remove or not states from this line and you're good.

quartata commented 6 years ago

I think most of us agree that we should keep CI on blacklist items (especially considering we do actually have tests for rogue regexes), so this should probably be closed for now.

teward commented 6 years ago

We have requests that arent compliant with regex format AND we need to CI test new blacklist or watchlists because they break Python or such.

-1 on the request to not run CI for blacklist/watchlist requests.

On Nov 25, 2017 18:28, "quartata" notifications@github.com wrote:

I think most of us agree that we should keep CI on blacklist items (especially considering we do actually have tests for rogue regexes), so this should probably be closed.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Charcoal-SE/SmokeDetector/issues/828#issuecomment-346973028, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUBENugqocyGn2_rgtPaAB9jPap_8-Yks5s6KKYgaJpZM4N3PwI .[image: Web Bug from https://github.com/notifications/beacon/AAUBELO46Q5Aj_Ysz4WVlgOjBwponjsWks5s6KKYgaJpZM4N3PwI.gif] {"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/Charcoal-SE/SmokeDetector","title":"Charcoal-SE/ SmokeDetector","subtitle":"GitHub repository","main_image_url":" https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/Charcoal-SE/SmokeDetector" }},"updates":{"snippets":[{"icon":"PERSON","message":"@quartata in #828: I think most of us agree that we should keep CI on blacklist items (especially considering we do actually have tests for rogue regexes), so this should probably be closed."}],"action":{"name":"View Issue","url":" https://github.com/Charcoal-SE/SmokeDetector/issues/828#issuecomment- 346973028"}}}

quartata commented 6 years ago

Precisely. A minute's wait isn't a huge deal (especially since currently it's still blacklisted locally anyways).

Undo1 commented 6 years ago

Kinda late, but 100% agree that we should be keeping CI on blacklist PRs.