Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 GitHub Actions Reporter #11320
feat: Add GitHub Actions Reporter #11320
Changes from 12 commits
4b441a1
6b299e2
b6cb64b
48e2778
195062d
808bd5c
5c4884f
97a7b1c
fc6dd30
6289d6a
5ab275e
ec6eb0d
a35a0d9
e72e447
20a6562
d8c0454
c7c4ac0
bb4bbe6
e4d5c01
1ae84fb
8b5414f
c963d7a
0461d49
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
question - should the reporter care? I'm thinking it makes more sense for the reporter to print when it's active, regardless of env. Then it's up to some other part of the code whether to activate it or not
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.
Ah, so not even having a more generic
isCI
check? 🤔I was kinda thinking the reporter could be enabled by default, since GH is so pervasive -- adding a bit of OOTB delight to everyone's developer experience 😄
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.
sure, but that's separate. We only enable coverage reporter of coverage is active (https://github.com/facebook/jest/blob/a20bd2c31e126fc998c2407cfc6c1ecf39ead709/packages/jest-core/src/TestScheduler.ts#L350-L381) we should do the same for a GH reporter. The reporter itself should always print if it's active, and the env detection should deicide to enable the reporter or not
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.
Gotcha! I'll remove the GHA check altogether then 👍
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.
e4d5c01
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.
For the defaults, should I add something like this to the PR?
Patch
Or should that be done in a follow-up?
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.
let's do that in a follow-up!
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.
Feel free to open up a PR when ready, btw. My main concern is if somebody wants to turn it off, that might be weird. But let's see! 🙂
Regardless, we should add
'github-actions'
as a recognized string in thereporters
array (likedefault
is)