Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

CourtbotError Functionality Expansion #33

Merged
merged 1 commit into from May 4, 2017
Merged

Conversation

chriswsh
Copy link
Collaborator

@chriswsh chriswsh commented May 4, 2017

Part 1 of 2

I hope splitting these up makes code review easier.

Functionality

  1. Added static wrap() utility function to wrap non-CourtbotErrors into CourtbotErrors. (This should allow updating all functions in events.js to pipakin's simpler pattern, in Part 2. The issue right now seems to be that unit testing is inconsistently enforcing functionality that's been moved to wrap(). My bad.)

Coding Practices

  1. changed class name from courtbotError to CourtbotError --> constructor functions typically start with capital letters
  2. Changed CourtbotError.case to CourtbotError.casenumber to avoid potential reserved word conflicts.
  3. Added backwards compatibility for CourtbotError.case; however, case is no longer an enumerable property. (Can we get away with dropping case?)

* Adding wrap() static method
* using better coding practices
@pipakin pipakin merged commit ef44a7e into master May 4, 2017
@pipakin pipakin deleted the CourtbotError-Wrapping branch May 4, 2017 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants