Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: add missing exports and tests for new APIs #495

Merged
merged 4 commits into from
Jul 27, 2020

Conversation

ctavan
Copy link
Member

@ctavan ctavan commented Jul 27, 2020

The new APIs were still missing from some environment-specific tests. We
should definitely consider consolidating these environment-specific test
cases somehow, this currently requires way too much manual work and it's
too easy to miss certain places that need an update.

ctavan added 3 commits July 27, 2020 11:15
The new APIs were still missing from some environment-specific tests. We
should definitely consider consolidating these environment-specific test
cases somehow, this currently requires way too much manual work and it's
too easy to miss certain places that need an update.
@ctavan ctavan marked this pull request as ready for review July 27, 2020 09:30
@ctavan ctavan requested a review from broofa July 27, 2020 09:30
Copy link
Member

@broofa broofa left a comment

Choose a reason for hiding this comment

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

Agreed! It'd be nice to have a single location for this stuff. Maybe a build step of some sort that generates the source? (Seems complicated, though... not sure how I really feel about that.)

I confess to somewhat deliberately ignoring these when doing the first round of new tests. Dealing with BrowserStack was enough of a hassle I deliberately ignored this side of things. Thanks for picking these up.

@ctavan
Copy link
Member Author

ctavan commented Jul 27, 2020

Agreed! It'd be nice to have a single location for this stuff. Maybe a build step of some sort that generates the source? (Seems complicated, though... not sure how I really feel about that.)

I'm also not entirely sure how a solution should look like. I have created #496 to explore this further. I think it's not super urgent but something we should attack at some point to keep this module maintainable.

I confess to somewhat deliberately ignoring these when doing the first round of new tests. Dealing with BrowserStack was enough of a hassle I deliberately ignored this side of things. Thanks for picking these up.

I feel you 😉

@ctavan ctavan merged commit 681e1da into master Jul 27, 2020
@ctavan ctavan deleted the add-missing-exports-and-tests branch July 27, 2020 15:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants