opal / opal-browser

Browser support for Opal.
MIT License
115 stars 36 forks source link

Work on updating specs (and a few useful features) #78

Closed hmdne closed 5 years ago

hmdne commented 5 years ago

This is a part of my ongoing work on modernizing the codebase.

On a sidenote, I think that the return of promises may at least be broken in a browser runner - I can't trigger a failure in async code. Unfortunately I lack the ability to debug this.

hmdne commented 5 years ago

I think I can do nothing more about BrowserStack tests. There's probably "BS_AUTHKEY" environment variable missing, that's what I understand from this error message:

*** Error: Atleast one argument is required!

I tested the BrowserStackLocal application locally and it only triggers this message when there's no key provided.

hmdne commented 5 years ago

I know I could have broken it all up to individual pull requests, but I hope it doesn't make your work harder (it's much easier for me that way). I try to describe the commits as well as I can.

elia commented 5 years ago

I know I could have broken it all up to individual pull requests, but I hope it doesn't make your work harder (it's much easier for me that way). I try to describe the commits as well as I can.

No problem, I'll review the commit one by one (as I usually do) it will just take time. I think I'll do multiple reviews so you can get early feedback 👍

hmdne commented 5 years ago

Thank you for this review. I fixed all the problems you mentioned.