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

All throw's use Error instances #2160

Merged
merged 1 commit into from
Oct 17, 2016
Merged

All throw's use Error instances #2160

merged 1 commit into from
Oct 17, 2016

Conversation

wimrijnders
Copy link
Contributor

Throw statements shouldn't be using bare strings; Motivation.

Copy link
Contributor

@yotamberk yotamberk left a comment

Choose a reason for hiding this comment

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

@mojoaxel These kind of style changes are legit to your taste? This is only in the Graph3d so I don't know if you meant that these style fixes should be across libs or per lib.

@wimrijnders
Copy link
Contributor Author

I don't consider this a style change. This improves the way that the code interacts with the browser, which is essential for e.g. debugging. Please read the motivation link.

Also, I have no intention of changing anything outside of Graph3d. If you can agree with the motivation, perhaps an effort can be made for globally changing all throw's. But I'm not sure that I want to do it.

@yotamberk
Copy link
Contributor

I understand and agree with this change. I just want to make sure with @mojoaxel that we are on the same mindset. I'd like his opinion before we merge it.

@wimrijnders
Copy link
Contributor Author

wimrijnders commented Oct 17, 2016

Hokay. I'm preparing PR5 in the meantime.

@mojoaxel
Copy link
Member

I also did not consider this a style change. This is good and useful.
The only problem I see is that it maight be considered a breaking change on some browsers, but I don't think that is realy a problem. So let's go for it!

@mojoaxel mojoaxel merged commit 85222bf into almende:develop Oct 17, 2016
@wimrijnders wimrijnders deleted the PR4 branch October 18, 2016 04:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants