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

Fixes #1241 Replacing instances of console with logger #1242

Merged
merged 3 commits into from
Dec 8, 2021
Merged

Fixes #1241 Replacing instances of console with logger #1242

merged 3 commits into from
Dec 8, 2021

Conversation

wongjas
Copy link
Member

@wongjas wongjas commented Dec 8, 2021

Summary

#1241 Replace instances of console.log where you have access to the built-in Bolt logger

Requirements (place an x in each [ ])

@wongjas wongjas self-assigned this Dec 8, 2021
@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #1242 (12e711b) into main (2b63d35) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1242   +/-   ##
=======================================
  Coverage   73.22%   73.22%           
=======================================
  Files          17       17           
  Lines        1438     1438           
  Branches      431      431           
=======================================
  Hits         1053     1053           
  Misses        300      300           
  Partials       85       85           

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 2b63d35...12e711b. Read the comment docs.

try {
const result = await client.chat.postMessage({
channel: welcomeChannelId,
text: `Welcome to the team, <@${event.user.id}>! 🎉 You can introduce yourself in this channel.`
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this change is correct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, I think you're right, I was testing with the wrong event! I was going to check the doc but there isn't a full example there either: https://api.slack.com/events/team_join.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I just tested it again to be sure and reverted the change now.

@seratch seratch added the docs M-T: Documentation work only label Dec 8, 2021
@seratch seratch added this to the 3.9.0 milestone Dec 8, 2021
Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

LGTM

@seratch
Copy link
Member

seratch commented Dec 8, 2021

@srajiang If this PR looks good to you too, you can "squash and merge" this tomorrow!

@srajiang srajiang merged commit 9658caa into slackapi:main Dec 8, 2021
Copy link
Member

@srajiang srajiang left a comment

Choose a reason for hiding this comment

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

Nice job with these @wongjas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs M-T: Documentation work only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants