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

feat: async signingSecret on ExpressReceiver #877

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

gmathieu
Copy link
Contributor

@gmathieu gmathieu commented Apr 13, 2021

Summary

Add async support to the signingSecret for the ExpressReceiver. This is useful when using secret managers (ex. https://github.com/googleapis/nodejs-secret-manager).

Requirements

@gitwave gitwave bot added the untriaged label Apr 13, 2021
@codecov
Copy link

codecov bot commented Apr 13, 2021

Codecov Report

Merging #877 (03a3e03) into main (23b4f80) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 03a3e03 differs from pull request most recent head 59663bd. Consider uploading reports for the commit 59663bd to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #877      +/-   ##
==========================================
+ Coverage   66.08%   66.11%   +0.02%     
==========================================
  Files          13       13              
  Lines        1200     1201       +1     
  Branches      353      354       +1     
==========================================
+ Hits          793      794       +1     
  Misses        338      338              
  Partials       69       69              
Impacted Files Coverage Δ
src/receivers/ExpressReceiver.ts 67.07% <100.00%> (+0.20%) ⬆️

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 23b4f80...59663bd. Read the comment docs.

@gmathieu
Copy link
Contributor Author

@seratch quick follow up on #875. I can do without but this would make code a lot cleaner and more robust. I imagine other receivers could benefit from this pattern as well. Thanks for your thoughts

@mwbrooks mwbrooks added enhancement M-T: A feature request for new functionality and removed untriaged labels Apr 13, 2021
@mwbrooks
Copy link
Member

Hey @gmathieu 👋🏻

I saw #875. Thanks for taking the time to create that pull request! While we didn't merge it, I'm glad to see we have a tight solution for developers while not adding more maintenance to our project. Definitely appreciate your understanding with it too!

I think this pull request is a fair enhancement. I've added a few maintainers as reviewers, so that someone can take a deeper look!

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.

Thanks for suggesting this enhancement! Looks great to me 👍

@seratch seratch added this to the 3.4.0 milestone Apr 13, 2021
@gmathieu gmathieu closed this Apr 14, 2021
@gmathieu gmathieu deleted the async_signing_secret branch April 14, 2021 15:29
@mwbrooks
Copy link
Member

Hmm... it looks like this pull request was ✅ approved but 🚫 closed before it was ⬇️ merged. 🙃

Since the branch was deleted, I'm not sure if we can restore the existing PR.

@gmathieu would you mind pushing the branch back up? It may allow you to re-open this PR. Otherwise, we'll need to open another (just reference this PR for simplicity).

@gmathieu gmathieu restored the async_signing_secret branch April 14, 2021 18:53
@gmathieu gmathieu reopened this Apr 14, 2021
@gmathieu
Copy link
Contributor Author

@mwbrooks sorry about that. Not sure what happened

@mwbrooks mwbrooks merged commit 263902a into slackapi:main Apr 14, 2021
@mwbrooks
Copy link
Member

@gmathieu No worries! I was a little confused too but it's great that GitHub was able to gracefully recover the pull request when you restored the branch. 🎉

Merged in your work. Thanks a bunch for the contribution! 🙇🏻

@gmathieu gmathieu deleted the async_signing_secret branch April 14, 2021 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality semver:minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants