Skip to content
This repository was archived by the owner on Sep 19, 2018. It is now read-only.

Add test_on and test_onload convenience functions.#108

Open
jgraham wants to merge 1 commit into
masterfrom
jgraham/test_on
Open

Add test_on and test_onload convenience functions.#108
jgraham wants to merge 1 commit into
masterfrom
jgraham/test_on

Conversation

@jgraham

@jgraham jgraham commented Feb 24, 2015

Copy link
Copy Markdown
Member

This change is Reviewable

@hoppipolla-critic-bot

Copy link
Copy Markdown

Critic review: https://critic.hoppipolla.co.uk/r/4091

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@hallvors

Copy link
Copy Markdown

Isn't the order of arguments a little..odd? It's closely related to addEventListener which has event type, function - and here we suddenly reverse it to function, object, type ..?

test_on(window, 'DOMContentLoaded', function(){ ... }, 'Test ...')

I would find easier to remember by association.

@jgraham

jgraham commented Feb 25, 2015

Copy link
Copy Markdown
Member Author

So the reasoning is that it matches all the other test* functions that always take the test function as their first argument (did that sentence make sense). I agree it feels a bit odd in this case though.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants