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

async triggers #348

Merged
merged 4 commits into from
Jan 27, 2021
Merged

async triggers #348

merged 4 commits into from
Jan 27, 2021

Conversation

gagoar
Copy link
Owner

@gagoar gagoar commented Jan 25, 2021

Issue Reference: #213

Description

all triggers are now async by default.

Motivation and Context

As a step to enable #346 and keep refactoring without changing functionality for #213 I've made all. the interfaces for the triggers be async.

note: Sadly I couldn't replace as much code as I wanted with the generated guards. but this will be solved when we are able to refactor the rules shape.

for now, we will have to keep the function isValidRawRule around given that it is more expressive than the ones autogenerated.

How Has This Been Tested?

unit tests.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #348 (95020f2) into master (8aaf8b4) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #348   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          349       342    -7     
  Branches        61        51   -10     
=========================================
- Hits           349       342    -7     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/index.ts 100.00% <100.00%> (ø)
src/rules.ts 100.00% <100.00%> (ø)

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 8aaf8b4...95020f2. Read the comment docs.


module.exports = {
globals: {
"ts-jest": {
'ts-jest': {
Copy link
Owner Author

Choose a reason for hiding this comment

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

just eslint --auto-fix

collectCoverage: true,
collectCoverageFrom: ["src/*.ts", "!./index.ts"],
coverageDirectory: "./coverage/",
collectCoverageFrom: ['src/*.ts', '!./index.ts', '!src/rules.guard.ts'],
Copy link
Owner Author

Choose a reason for hiding this comment

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

we don't want to collect coverage from an autogenerated file.

@@ -102,7 +102,6 @@
"author": "Gago <[email protected]>",
"dependencies": {
"@actions/core": "1.2.6",
"@github/memoize": "^1.0.1",
Copy link
Owner Author

Choose a reason for hiding this comment

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

we have the actual files instead of the library anyway.

@@ -137,6 +136,7 @@
"prettier-eslint": "12.0.0",
"prettier-eslint-cli": "5.0.0",
"pretty-quick": "3.1.0",
"ts-auto-guard": "1.0.0-alpha.13",
Copy link
Owner Author

Choose a reason for hiding this comment

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

this is what I've used to generate the type guards

Copy link
Collaborator

@cyamonide cyamonide 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 🚀

@gagoar gagoar force-pushed the gg/async-triggers branch from c263e75 to 5c45e0e Compare January 27, 2021 03:25
@gagoar gagoar merged commit 3066dfe into master Jan 27, 2021
@gagoar gagoar deleted the gg/async-triggers branch January 27, 2021 03:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants