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

Making Slack alert documentation consistent #111

Merged
merged 1 commit into from
Dec 23, 2020

Conversation

peterthesling
Copy link
Contributor

@peterthesling peterthesling commented Dec 17, 2020

See the following documentation:
image

Renaming everything to channels.

@codecov-io
Copy link

codecov-io commented Dec 17, 2020

Codecov Report

Merging #111 (3ba9844) into master (6870644) will increase coverage by 70.15%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #111       +/-   ##
===========================================
+ Coverage    1.78%   71.94%   +70.15%     
===========================================
  Files          16       31       +15     
  Lines         672     1700     +1028     
===========================================
+ Hits           12     1223     +1211     
+ Misses        660      477      -183     
Impacted Files Coverage Δ
pipelines/whale/extractor/metric_runner.py 81.70% <ø> (ø)
cli/src/warehouse/whutils.rs
cli/src/serialization.rs
cli/src/warehouse/generic.rs
cli/src/warehouse/glue.rs
cli/src/warehouse/metadatasource.rs
cli/src/warehouse/bigquery.rs
cli/src/main.rs
cli/src/warehouse/hive.rs
cli/src/warehouse/buildscript.rs
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee5618c...3ba9844. Read the comment docs.

@peterthesling peterthesling force-pushed the pthesling/slack-alert-documentation branch 3 times, most recently from a15670b to 3ba9844 Compare December 18, 2020 20:22
@rsyi
Copy link
Owner

rsyi commented Dec 18, 2020

Hmm. I actually like it as slack, in case in the future we end up building support for other clients, like discord. What was the reasoning here? :)

@peterthesling peterthesling force-pushed the pthesling/slack-alert-documentation branch 5 times, most recently from ca045be to 47015ce Compare December 23, 2020 16:49
@peterthesling
Copy link
Contributor Author

Hmm. I actually like it as slack, in case in the future we end up building support for other clients, like discord. What was the reasoning here? :)

So the main purpose of the PR was to make it consistent. I was missing the context / didn't think of adding other clients, so all good.

@peterthesling peterthesling force-pushed the pthesling/slack-alert-documentation branch from 47015ce to 47816db Compare December 23, 2020 17:00
@rsyi
Copy link
Owner

rsyi commented Dec 23, 2020

Ah got it. This is awesome @peterthesling thanks :) Merging!

@rsyi rsyi merged commit 6b4fed9 into rsyi:master Dec 23, 2020
@peterthesling peterthesling deleted the pthesling/slack-alert-documentation branch December 23, 2020 22:37
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.

3 participants