Update log.js to support async reporter plugins by steveszc · Pull Request #71 · unifiedjs/unified-engine · GitHub
Skip to content

Update log.js to support async reporter plugins - #71

Merged
wooorm merged 4 commits into
unifiedjs:mainfrom
steveszc:patch-1
Nov 19, 2022
Merged

Update log.js to support async reporter plugins#71
wooorm merged 4 commits into
unifiedjs:mainfrom
steveszc:patch-1

Conversation

@steveszc

@steveszc steveszc commented Nov 16, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Async reporters can be easily supported by awaiting the reporter function.

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • If applicable, I’ve added docs and tests

Description of changes

Log.js can easily support sync reporter plugins by awaiting the reporter function. Currently the reporter function is called as a sync function.

Example: vfile-reporter-junit
https://github.com/kellyselden/vfile-reporter-junit/blob/master/index.js

@steveszc
Update log.js to support async reporter plugins
6c4452b 
Async reporters can be easily supported by `await`ing the reporter function.
@github-actions github-actions Bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Nov 16, 2022
@steveszc

steveszc commented Nov 16, 2022

Copy link
Copy Markdown
Contributor Author

remcohaszing
remcohaszing requested changes Nov 16, 2022
View reviewed changes

@remcohaszing remcohaszing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see why this is useful. The types need to be updated though.

ESLint 8 also added support for async reporters.

@steveszc

steveszc commented Nov 16, 2022

Copy link
Copy Markdown
Contributor Author

I am getting type errors on main when I run npm test.

Not sure I can get this PR passing until main is working...

@wooorm

wooorm commented Nov 17, 2022

Copy link
Copy Markdown
Member

Can you add a test? (https://github.com/unifiedjs/unified-engine/blob/main/test/reporting.js)

This should perhaps also be added to https://github.com/vfile/vfile/blob/03efac7cc3a67fe33bb7a0f1de29e17be322549e/lib/index.js#L46

@codecov-commenter

codecov-commenter commented Nov 17, 2022

Copy link
Copy Markdown

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (e7fc5f9) compared to base (398ebe2).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head e7fc5f9 differs from pull request most recent head cf12f6d. Consider uploading reports for the commit cf12f6d to get more accurate results

Additional details and impacted files 
@@            Coverage Diff            @@
##              main       #71   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        23           
  Lines         2555      2555           
=========================================
  Hits          2555      2555
Impacted Files Coverage Δ
lib/file-set-pipeline/log.js 100.00% <100.00%> (ø)
lib/index.js 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@steveszc

steveszc commented Nov 17, 2022

Copy link
Copy Markdown
Contributor Author

@wooorm Test added and passing

@wooorm
wooorm merged commit 245bd28 into unifiedjs:main Nov 19, 2022
@github-actions

This comment has been minimized.

Sign in to view
@wooorm wooorm added 🦋 type/enhancement This is great to have 💪 phase/solved Post is done labels Nov 19, 2022
@github-actions github-actions Bot removed the 🤞 phase/open Post is being triaged manually label Nov 19, 2022
@wooorm

wooorm commented Nov 19, 2022

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@remcohaszing remcohaszing remcohaszing approved these changes
@wooorm wooorm wooorm approved these changes

Assignees

No one assigned

Labels

💪 phase/solved Post is done 🦋 type/enhancement This is great to have

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@steveszc @wooorm @codecov-commenter @remcohaszing