search

w3c / encrypted-media

Encrypted Media Extensions
https://w3c.github.io/encrypted-media/
Other
180 stars 80 forks source link

Use undefined union for MediaKeyStatusMap#get() #477

Closed saschanaz closed 3 years ago

saschanaz commented 3 years ago

Thanks to https://github.com/heycam/webidl/pull/906.

Preview | Diff

joeyparrish commented 3 years ago

It was hard to find the functional change among all the cosmetic changes, but the change looks good to me. In the future, could you separate functional and cosmetic changes to make it easier to read the diff?

saschanaz commented 3 years ago

@joeyparrish Sorry to make the review hard, but fortunately there is "Hide whitespace changes" feature in GitHub for you 😉

image

joeyparrish commented 3 years ago

That's useful, but GitHub is by no means the only way we look at diffs in a repo's history. I appreciate the whitespace cleanup, but I just prefer to have cosmetic commits separate from functional ones for clarity. That was a community rule on the first OSS project I ever contributed to, and it still feels valuable to me after all these years. Or maybe I'm just getting old. :-)

saschanaz commented 3 years ago

Got it, I'll make sure to separate them next time 🙏

BTW I forgot to update the index.html here, should I open a new PR?

joeyparrish commented 3 years ago

After #479 is merged, nobody will ever have to manually regenerate index.html ever again. 🎉 🥳 🎈