Skip to content

Allow running a portion of binaryen test suite#2286

Merged
quantum5 merged 5 commits into
WebAssembly:masterfrom
quantum5:check-filter
Aug 7, 2019
Merged

Allow running a portion of binaryen test suite#2286
quantum5 merged 5 commits into
WebAssembly:masterfrom
quantum5:check-filter

Conversation

@quantum5

@quantum5 quantum5 commented Aug 6, 2019

Copy link
Copy Markdown
Member

No description provided.

@tlively

tlively commented Aug 6, 2019

Copy link
Copy Markdown
Member

Can you add some usage examples in a comment at the top of check.py?

@sbc100 sbc100 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 have a very similar change I've been working on. I like your version better though.

Could we do that same for auto_update_tests.py perhaps in a followup?

Comment thread check.py Outdated

@kripken kripken 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.

Lgtm with examples as @tlively said. Maybe add examples to the readme too, or at least a reference from one to the other place.

@aheejin

aheejin commented Aug 6, 2019

Copy link
Copy Markdown
Member

Can you add some usage examples in a comment at the top of check.py?

How about having a proper help message about the usage for -h or something? In both check.py and auto_update_test.py. Thanks!

@jgravelle-google jgravelle-google left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great change! +1 for some light documentation.

Comment thread check.py Outdated
@quantum5 quantum5 merged commit 4c9cfeb into WebAssembly:master Aug 7, 2019
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.

6 participants