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

test: separate internal/external #1680

Merged
merged 1 commit into from
Aug 14, 2021
Merged

Conversation

theoludwig
Copy link
Member

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix
[ ] New feature
[x] Other, please explain: Tests

What changes did you make? (Give an overview)

Separate the tests in 2 scripts : test-internal and test-external.

From @voxpelli :

That way the badging situation will be easier to decipher – for maintainers as well as users and contributors.

👍

Which issue (if any) does this pull request address?

Ref: #1663 (comment)

Copy link
Member

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

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

LGTM 👍

package.json Outdated Show resolved Hide resolved
@theoludwig
Copy link
Member Author

Before merging, we need to update the branch protection rules with these new workflows.

@voxpelli
Copy link
Member

I'm 👍 on the changes

@voxpelli
Copy link
Member

Random note: After this PR we should add 16.x to the test matrix

Copy link
Member

@LinusU LinusU left a comment

Choose a reason for hiding this comment

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

Neat 👍

@theoludwig theoludwig force-pushed the test/separate-internal-external branch from 3fe4872 to f536aed Compare July 23, 2021 16:52
@theoludwig
Copy link
Member Author

Random note: After this PR we should add 16.x to the test matrix

Agreed. I already added 16.x to the test matrix, is it fine or we should wait to do that after this PR ?

@voxpelli
Copy link
Member

Random note: After this PR we should add 16.x to the test matrix

Agreed. I already added 16.x to the test matrix, is it fine or we should wait to do that after this PR ?

It’s fine I would say :)

@voxpelli
Copy link
Member

@divlo Something else you want to get fixed before merging?

@theoludwig
Copy link
Member Author

@divlo Something else you want to get fixed before merging?

Yes, test external with Node.js v16 is failing.
This is one of @feross repo that fails, see: feross/bitmidi.com#116.

@voxpelli
Copy link
Member

@divlo lets remove 16.x from this PR then, merge this PR, and then open a new one for that specific one :) This fix is too good to sit around here ;)

@theoludwig theoludwig force-pushed the test/separate-internal-external branch from b1ce97d to 8a7ec34 Compare August 14, 2021 18:52
@theoludwig theoludwig merged commit 0aacac1 into master Aug 14, 2021
@theoludwig theoludwig deleted the test/separate-internal-external branch August 14, 2021 18:52
@theoludwig
Copy link
Member Author

@divlo lets remove 16.x from this PR then, merge this PR, and then open a new one for that specific one :) This fix is too good to sit around here ;)

Let's do that! 😎
I opened a separate PR to add Node.js v16 : #1703

@theoludwig theoludwig mentioned this pull request Aug 14, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants