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

Feature/#49 - Add an option to hide status cancelled events #51

Merged
merged 2 commits into from
Jun 30, 2020

Conversation

AHaliq
Copy link
Contributor

@AHaliq AHaliq commented Jun 24, 2020

Description of the task

Detect cancel events via status failure and description text and dont emit status when encountered

How to test

./notabot check --config=test/notabot.json --secrets=test/secrets.json mock_payloads/status.cancelled_test.json

References

lib/slack.ml Outdated Show resolved Hide resolved
lib/slack.ml Outdated Show resolved Hide resolved
lib/slack.ml Outdated Show resolved Hide resolved
@Khady
Copy link
Contributor

Khady commented Jun 24, 2020

ok now you have something which works, but only with our current setup. Can you try to make the filtering of configurable?

lib/action.ml Outdated Show resolved Hide resolved
lib/config.ml Outdated Show resolved Hide resolved
@AHaliq AHaliq changed the title Feature/#49 Feature/#49 - Add an option to hide status cancelled events Jun 25, 2020
.gitignore Outdated Show resolved Hide resolved
lib/action.ml Outdated
let { state; description; _ } = notification in
match description, state with
| Some s, Failure
when Str.string_match (Str.regexp_case_fold "^\\(Build #[0-9]+ canceled by .+\\|Failed (exit status 255)\\)$") s 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it is better to document that we support buildkite only and are open to PR to add other CI systems. Or if we should have a configuration option to let the user write their own regexp.

Copy link
Contributor

@Khady Khady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

last small change and I think it should be good to go

lib/action.ml Outdated Show resolved Hide resolved
lib/action.ml Outdated Show resolved Hide resolved
@ygrek
Copy link
Contributor

ygrek commented Jun 26, 2020

please cleanup commits (e.g. squash into one)

added end of line regex

fix one time check for cancel event

added test and fixed regex

test: add cache for 78492c2467876259d787538d600cfa0b18a2b814

slack: add culprit sha when notification can't be handled

fix

call generate for each webhook, defined expected output for new test

refactor if to match

extract filter logic out of generate_status_notification and into a function in generate_notifications in action.ml

removed vscode settings

added suprress_cancelle_events config value

set supress to true

moved function

refactor return blank list if filter fail instead of per webhook filter

use Re instead of Str

compile regexp before usage

removed .vscode and move to personal global

left new line at end of gitignore

removed .vscode

refactor to use Option.default, set default to true, added documentation

filter merge commit of main branch in status events

make test_promote

change filter to is, invert bool

github actions: update apt packages
@Khady
Copy link
Contributor

Khady commented Jun 30, 2020

any reason this is still not merged?

@AHaliq AHaliq merged commit d3fdc46 into master Jun 30, 2020
@ygrek ygrek deleted the feature/#49 branch June 30, 2020 21:12
yasunariw pushed a commit that referenced this pull request Nov 30, 2021
Feature/#49 - Add an option to hide status cancelled events
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.

Add an option to hide status cancel events
3 participants