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: Add new type list_id, builtins for Add/Remove Reactions and many List-related functions #361

Merged
merged 4 commits into from
Sep 6, 2024

Conversation

filmaj
Copy link
Contributor

@filmaj filmaj commented Sep 6, 2024

  • updates instructions for how to generate built-in function definitions using updated functions.list API endpoint
  • generation script will now throw an exception if it encounters a parameter type it is unfamiliar with
  • have generation script also run TypeScript type checking on generated files

…ctions, too.

also updates instructions for how to generate built-in function definitions using updated `functions.list` API endpoint
@filmaj filmaj added the semver:minor requires a minor version number bump label Sep 6, 2024
@filmaj filmaj self-assigned this Sep 6, 2024
@filmaj filmaj requested a review from a team as a code owner September 6, 2024 14:40
Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

Looks good to me 💯

Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

Ohh wait just saw the CI is failing 🤔

… finds an unfamiliar parameter type. Update generated list functions with new list_id type.
@filmaj filmaj changed the title feat: Add builtins for Add/Remove Reactions and many List-related functions feat: Add new type list_id, builtins for Add/Remove Reactions and many List-related functions Sep 6, 2024
Copy link

codecov bot commented Sep 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.66%. Comparing base (2ab8d3b) to head (74e6d9e).

Additional details and impacted files
@@            Coverage Diff            @@
##           main     #361       +/-   ##
=========================================
+ Coverage      0   98.66%   +98.66%     
=========================================
  Files         0       73       +73     
  Lines         0     2997     +2997     
  Branches      0      147      +147     
=========================================
+ Hits          0     2957     +2957     
- Misses        0       40       +40     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

:shipit:

@@ -21,3 +21,5 @@ echo "Formatting Slack function files..."
deno fmt --quiet ../*.ts
echo "Linting Slack function files..."
deno lint --quiet ../*.ts
echo "Type-checking Slack function files..."
deno check --quiet ../*.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 💯 I thought this was already in there

@@ -73,8 +73,14 @@ const propertyToTypeScript = (
property: FunctionProperty,
): string => {
const typescript = [];
const sdkType = schemaTypeMap[property.type];
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice improvement 💯 makes the code clearer also

@filmaj filmaj merged commit a2f4ffa into main Sep 6, 2024
11 checks passed
@filmaj filmaj deleted the builtin-updates branch September 6, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor requires a minor version number bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants