Skip to content
This repository has been archived by the owner on Jan 17, 2019. It is now read-only.

Deal with no argument passed. #73

Merged
merged 3 commits into from Oct 6, 2016
Merged

Conversation

Deuteu
Copy link
Contributor

@Deuteu Deuteu commented Oct 2, 2016

cf commit description

@mnapoli
Copy link
Member

mnapoli commented Oct 3, 2016

Just to be sure I understand: if someone calls confirm() with no option then silently nothing will happen? It might be hard to understand/debug.

@Deuteu
Copy link
Contributor Author

Deuteu commented Oct 3, 2016

You understood the behaviour.
You prefer to throw error ?

If error thrown would be great to also do it for the presence of confirmation-modal no ?

@mnapoli
Copy link
Member

mnapoli commented Oct 3, 2016

Yes in case there's something wrong there should be an error IMO

If error thrown would be great to also do it for the presence of confirmation-modal no ?

I'm not sure I understand?

@Deuteu
Copy link
Contributor Author

Deuteu commented Oct 3, 2016

If we throw error for the absence of options because it easier to track for debug, it seems logical to me to also not exit whitout error when there is allready a .confirmation-modal (current behaviour at jquery.confirm.js#L45).

This is not the purpose of that PR but more a proposition for a common behaviour for the different premature exits if we introduce error in this case.

@Deuteu
Copy link
Contributor Author

Deuteu commented Oct 3, 2016

So for this PR it's should be good to go now.

I might do an other one for what I was talking about earlier.

@mnapoli
Copy link
Member

mnapoli commented Oct 6, 2016

👍

@mnapoli mnapoli merged commit 88d1c71 into myclabs:master Oct 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants