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

Already on GitHub? Sign in to your account

Throw error when flot.time plugin is missing. #55

Merged
merged 2 commits into from Jul 11, 2012

Conversation

Projects
None yet
2 participants
Contributor

yaelelmatad commented Jul 9, 2012

Fixes issue 709 on google code.

@ghost ghost assigned dnschnur Jul 10, 2012

That's the correct way to throw the exception, and you've picked the right place for this check. Some comments:

  • Use == when checking opts.mode. The === operator checks for physical equality; whether two values are literally the same object at the same memory address. In this case it actually works most of the time, because browsers try to save memory by re-using short strings, but in theory it's not supposed to. The == operator checks for value equality - whether both strings have the same four characters - which is what you really want here.
  • Instead of using typeof, just check whether axis.tickGenerator == undefined. Note the use of == rather than === here. In many situations that's bad practice, since == will return true for both undefined and null. But in this case it's actually better behavior, since neither value should be present.
  • You have some tabs at the start of your changed lines; convert them to four-space indents. This is super minor, and I prefer tabs personally, but whenever you're working on someone else's codebase it's best to follow their style conventions. Ole works with Python, so he probably had his IDE set to spaces when he wrote this code :)

I'll merge once you make those changes.

@yaelelmatad yaelelmatad Edited request for Ticket 709, Deprecation Mesg
We changed the === to == and removed the typeof operator.  We also changed the tabs to spaces.
8beb199

@dnschnur dnschnur added a commit that referenced this pull request Jul 11, 2012

@dnschnur dnschnur Merge pull request #55 from yaelelmatad/issue-709-deprecation-msg
Throw error when flot.time plugin is missing.
65b05c6

@dnschnur dnschnur merged commit 65b05c6 into flot:master Jul 11, 2012

Owner

dnschnur commented Jul 11, 2012

Looks good; thank you!

@dnschnur dnschnur added a commit that referenced this pull request Jul 11, 2012

@dnschnur dnschnur Added note and credits for the merge from pull request #55, and unexp…
…ected fixing of issue #541.
b57542f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment