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

Refactor Client #270

Merged
merged 1 commit into from
Jan 16, 2015
Merged

Refactor Client #270

merged 1 commit into from
Jan 16, 2015

Conversation

nateberkopec
Copy link
Contributor

  • remove redundant excluded environment check
  • cleanup methods, esp. send
  • remove redundant private
  • remove unnecessary return statements, variable assignments

else
raise Error.new("Unknown transport scheme '#{self.configuration.scheme}'")
raise "Unknown transport scheme '#{self.configuration.scheme}'"
Copy link
Contributor

Choose a reason for hiding this comment

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

This will cause it to raise RuntimeError instead of Raven::Error. It seems this code should be:

raise Error, "Unknown transport scheme '#{self.configuration.scheme}'"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ai! Yes, definitely need to fix.

@nateberkopec
Copy link
Contributor Author

Renaming should_send to should_capture is a fairly major API change. We may need to get serious about deprecation warnings. I know it's 0.X and all but I dunno.

@nateberkopec nateberkopec added this to the 0.13.0 milestone Jan 16, 2015
+ remove redundant excluded environment check
+ cleanup methods, esp. send
+ remove redundant private
+ remove unnecessary return statements, variable assignments
+ rename should_send/should_capture
@nateberkopec
Copy link
Contributor Author

Deciding I'm going to roll with it for now. I'm going to try to keep patchlevel releases non-breaking, but minors may break things for as long as we're at 0.x.

nateberkopec added a commit that referenced this pull request Jan 16, 2015
@nateberkopec nateberkopec merged commit dbb0a8d into getsentry:master Jan 16, 2015
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.

2 participants