Skip to content

Conversation

@MikeHolman
Copy link
Contributor

add jenkins support for .wasm files. Note, that this is the binary format for WebAssembly, and we have a couple of these files in the wasm branch. This is causing test failures in #60

@msftclas
Copy link

Hi @MikeHolman, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Michael Holman). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@MikeHolman
Copy link
Contributor Author

@dilijev, can you please review?

@MikeHolman
Copy link
Contributor Author

After talking with @dilijev I will include this in same PR on top of rest of wasm changes.

@MikeHolman MikeHolman closed this Jan 13, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're here, you might change the grep -v -E "..." pattern to match what's in the other file.

Namely, change this line to

git diff --name-only `git merge-base origin/master HEAD` HEAD |
    grep -v -E 'test/.*\\.js$' |
    grep -v -E '\\.cmd$' |
    grep -v -E '\\.wasm$' |
    grep -v -E '\\.baseline$' |
    xargs -I % ./jenkins.check_file_eol.sh % 

That will make it easier to make future changes to this list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, that definitely is easier to mentally parse.

@dilijev
Copy link
Contributor

dilijev commented Jan 13, 2016

@MikeHolman I made some comments here. When you integrate this commit with your new PR, can you make these changes as well? Thanks.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants