Skip to content

Conversation

@woodsmc
Copy link
Collaborator

@woodsmc woodsmc commented Nov 18, 2025

Speaking to @lum1n0us I was suggesting that we change our process for accepting code contributions to require testing, unless there is a good reason not to have them. At the moment supplying a test is optional.

I suggest we should change this to be a requirement -> if you change code supply a test. We could have contributors point to existing tests which cover their contribution, or supply new testing. If the code change is super tiny, like formatting, or variable name change, then code's existing functionality is testing with the existing tests.

If the code contribution is larger or affects logic, fixes a bug etc, then we should require a test. In the case of a bug, a test case to reproduce the issue, and the fix, so we can re-run the test case to show the issue cannot be reproduced.

This is my first attempt at the text for this, it's a little all over the place. But it's a start. I'd welcome your thoughts on the idea.

…ions to make supplying a test manditory, except in exceptional situations.
@woodsmc
Copy link
Collaborator Author

woodsmc commented Nov 18, 2025

Yes, and following these guides, I'd have to add :

This is a minor change explain the change, and as such is except from addition test contribution.

Yes, the irony of me not adding this disclaimer is high,... ;p

Copy link
Collaborator

@lum1n0us lum1n0us left a comment

Choose a reason for hiding this comment

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

LGTM. hope to hear others' opinions.

Copy link
Collaborator

@loganek loganek left a comment

Choose a reason for hiding this comment

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

100% agree. I wonder if we should also update our PR template to include the checkbox about testing?

Copy link
Collaborator

@TianlongLiang TianlongLiang left a comment

Choose a reason for hiding this comment

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

Good idea 👍

@lum1n0us lum1n0us merged commit f6ce52f into bytecodealliance:main Nov 20, 2025
1 check passed
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.

4 participants