-
Notifications
You must be signed in to change notification settings - Fork 1
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
JSDoc, deno.jsonc
, return type tweaks; prep for JSR.io publish test
#8
Conversation
…ting a message boundary protocol implementation (it always contains a `getCLIFlags` method), removed deprecated nested options from deno.jsonc, changed coverage task to report detailed coverage.
"fmt": { | ||
"files": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The files
and options
keys are deprecated, and deno
would warn about using them, so removed them.
export const MessageBoundaryProtocol = function (args: string[]): Protocol { | ||
export const MessageBoundaryProtocol = function ( | ||
args: string[], | ||
): Required<Pick<Protocol, "getCLIFlags">> & Protocol { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The base Protocol
interface sets getCLIFlags
as an optional key. However, the Message Boundary protocol always returns this key, so tweaking the type to make that a required property.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8 +/- ##
==========================================
+ Coverage 91.93% 95.77% +3.83%
==========================================
Files 3 1 -2
Lines 62 71 +9
Branches 5 5
==========================================
+ Hits 57 68 +11
+ Misses 5 3 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome 💯
@@ -1,31 +1,39 @@ | |||
{ | |||
"$schema": "https://deno.land/x/deno/cli/schemas/config-file.v1.json", | |||
"name": "@slack/protocols", | |||
"version": "0.0.2-pre.2", | |||
"exports": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love this export implementation 💯
@WilliamBergamin i added a publish workflow for JSR, just based on what jsr's site says: https://jsr.io/docs/publishing-packages#publishing-from-github-actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good 💯 I think we can merge and fix forward if we encounter issues
Following the docs here to try to publish something to jsr.io: https://jsr.io/docs/publishing-packages
Also added an extra test to bump coverage up by a couple %, removing deprecated fields from
deno.jsonc
, tweaking JSdocs to be consistent, and made a slightly more accurate return type for the Message Boundary protocol.Post-Merge TODOs
?subdir
querystring parameter to the deno.land webhook