Fixes and polish for stable release#2128
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat-functions #2128 +/- ##
==================================================
+ Coverage 81.43% 81.58% +0.15%
==================================================
Files 19 19
Lines 1621 1640 +19
Branches 458 462 +4
==================================================
+ Hits 1320 1338 +18
Misses 194 194
- Partials 107 108 +1 ☔ View full report in Codecov by Sentry. |
misscoded
force-pushed
the
pre-stable-fixes
branch
from
ab80d52 to
9df1e68
Compare
misscoded
force-pushed
the
pre-stable-fixes
branch
from
9df1e68 to
f7966e7
Compare
zimeg
left a comment
zimeg
left a comment
There was a problem hiding this comment.
LGTM! Nice to see the stronger typings here 🙏 Left a few comments around imports but nothing blocking IMO
| } from './types'; | ||
| import processMiddleware from './middleware/process'; | ||
| import { CustomFunctionInitializationError } from './errors'; | ||
| import { CustomFunctionCompleteFailError, CustomFunctionCompleteSuccessError, CustomFunctionInitializationError } from './errors'; |
There was a problem hiding this comment.
We might want to just use the CustomFunctionInitializationError error here to not confuse initialization and runtime errors 🤔
| if (type === IncomingEventType.Action && context.functionExecutionId !== undefined) { | ||
| listenerArgs.complete = CustomFunction.createFunctionComplete(context, client); | ||
| listenerArgs.fail = CustomFunction.createFunctionFail(context, client); | ||
| listenerArgs.inputs = context.functionInputs; |
There was a problem hiding this comment.
Just sharing as an option, free to ignore- all listener arguments could be created and assigned with Object.assign if we wanted to reduce the number of CustomFunction exports
| import { Logger } from '@slack/logger'; | ||
| import { StringIndexed } from './helpers'; | ||
| import { SlackEventMiddlewareArgs } from './events'; | ||
| import { FunctionInputs, SlackEventMiddlewareArgs } from './events'; |
There was a problem hiding this comment.
Could these be imported from ./functions instead?
3 participants
Summary
Final preparations and polish prior to cutting a stable release. 💅🏼
inputsavailable by way ofContextfor interactivity events that are associated with a function's executionContextinterfacefunctionExecutionIdsomehow goes missing when using a utilityfeat-functions) PR feedback (TS fixes)actionpayloadRequirements (place an
xin each[ ])