labofsahil/github-event-dataset-2012 huggingface dataset:
1. You call `hasMaterial` (which called `getMaterial`) and then call `getMaterial` again yourself.  Just call `getMaterial` and rename isOldMaterialType to oldMaterial.  Then you don't need the second get.
2. Also, never use `if(oldMaterial)` unless you know you are working with a boolean.  Instead use `if(typeof oldMaterial === 'undefined')`
As above, perhaps `union` is better.
How about `specularBlendColor`?
No need to submit comments in duplicate.  And really we should have a guideline about submitting more comments than there is code...

Can you submit a patch for the seealso tag?  Or point me to an example of its usage, so I can make one?
Perhaps the caller is better at managing pending tasks, e.g., from frame-to-frame we want to remove tile requests that are out of view.

For more trivial users, can't the `TaskProcessor` know when a tasks completes, and then dequeue another one.  Or you feel that it is better behavior to not accept tasks we'd have to queue up since we can't provide a useful way to manage them?  I'm not sure I buy it, but it's also not in the scope of this pull request.
Between these and other uniforms, we could exceed the minimum maximum number of fragment shader uniforms, and the shader will fail to compile.  We are guaranteed 8 `sampler2D` and 16 `vec4` (anything less than a `vec4` counts as a `vec4`).

For robustness, we could switch to multi-pass.  Don't we already do that based on texture units?  Perhaps we should do it based on uniforms too?  Or put it on the roadmap under robustness.
Sure. I am going to merge this
MW supports this
There isn't a textContent property in android
Should be "animate" ... Maybe say:

"View.animate called before view %@ was ready. Will retry."
I would add a log.info to be clear?
Agreed, it was not refined.  I have your fixed in my latest local branch, works.
Let's change to "modeled after Underscore"
Seems like it would be more consistent to have the two tabs named `Regular` and `Async` (i.e. without the `splunkjs.` prefix). Thoughts?
is that even possible? It'd be pretty awesome..
I'm surprised that it isn't. It makes no sense for a range-based function which does not return a lazy range to accept an infinite range. It's a guaranteed infinite loop (or in some cases, a loop until `size_t.max` - neither is good). And given that it's a guaranteed to be a bug, I see no problem in making such a function stricter and making it disallow infinite ranges.
In C++'s STL, it is. I'm just following that. But, feel free to suggest another category if it's more fitting.
We should cross-reference takeExactly so it's clickable.
That does sound smarter. I'll correct it in another diff. BTW, "find" uses a similar scheme, but I'm not touching that.
You can have only the case "slice.emitter.y > display.viewableContentHeight", because we know that the slices are going down and ultimately will have a high Y.

The 4 tests for the 4 borders may create strange behavior, such as a rabbit sprite going up out of the top of the screen and never going down.
That's a long discussion we are having. Is it ok to just change license headers? it is a tricky question and probably best not to change them. I rather leave it as is and accept both versions.
can you explain why we no longer need the OpenEngSBModel here?
why not making this method private and as part of modeldescription if it's only used there?
Again, that should probably go away in a release version :)
didn't know that. and easier to read actually. commit coming.
This actor is only forwarding to a router, please replace directly with the router.
No need to check if the wrapped object responds to image since you're already enforcing the definition. Hence, no need for the yield.

```
def image
  normalized(profile.image)
end
```
in the current development cycle, we've agreed as a group that this behavior is acceptable. in fact, dev reload-unit-tests will automatically rebuild your db if it sees modified migrations.
As we get closer to final release, then yes - there's definitely a desire to use migrations the way they were intended to be used.
why couldn't some of these paths should use the resource :users?
We don't want to do real net connections in specs.  Did you mean to leave this on?
Why is this field no longer null (but still blank)?
We should be using `live`. Use `on` instead.

Should Togle not be Toggle?
How much of this data is necessary? For example, do we need `title` and `description`, and for that matter `sakai:indexed-fields`, if we're not using those features of this widget anymore?
Fixed. It didn't make the function any shorter though.
What's wrong with `auto` here? If we were talking about a function return type, that would be one thing, but I see no reason to use explicit types for local variables if it's not needed.
move to before the first use
Yup, will look into doing that.
Same here - use the path attribute instead of name
You could chain this together:

```
plot.getPlaceholder().bind('plothover', onHover).bind('plotselected', onPlotSelected). ...
```
I figured our search layout would have the search box with the button next to it, and the filters below would look like they're filtering the search results (and we might eventually have them trigger a search on change, or maybe add a filter button), but if you know how it's supposed to look in the end I can change the layout.
The current/latest one, that will futureproof us :)
thanks for adding an example about children search. i wonder if we can find a better example when search is needed. if you have a blog node with the blog posts being direct children, the best way to get them is having a blog document with a children mapping. in the past i have seen people coming from RDBMS have a tendency to use search for things where search is overhead.
I agree: ``subdomain of subdomains`` is more readable.
Why store the signature into the normalized object and delete it later?

I think is better to use json.signature directly after
I really prefer parens on this kind of compound statement.
Que nomezinho tosco, né?
@coreypurcell Please let me know what you want to do about this. Do you want it work just like the secret_token? Or should we just re-use the same secret_token for encrypting the SSNs?
that should be an unreachable edge case, the redirect url is cleared down as we redirect users to it so should not be in scope here
Same as before - rather than make this switch depending on if request is available or not, make it solely based on `USE_ABSOLUTE_URLS`.

If it's `True` and request isn't available raise an assertion error.
This is something that I personally really hate. Is it our convention? Is it something broadly accepted? I really think it makes the code a lot harder to read, it's too compacted and it hides the  if else structure. 

But as usual, if that's the convention, I'll accept it.
use lower came casing
Exactly - that's where you put it:

    git mv {public,assets}/stylesheets/whatever.less
I'll defer to @honestbleeps for that, since I don't have access to the subreddit which hosts RES's images. However, I could take care of spriting the mail icons together and making the CSS to save him the ten minutes.

While we're on the topic, there are a few other image URLs that should get updated: 

* other reference to the colorblind-friendly mail icon in `modules['styleTweaks'].go`
* vote sprite: http://thumbs.reddit.com/t5_2qlyl_0.png?v=8oarqkcswl255wrw3q1kyd74xrty50a7wr3z
* `#baconBit`: http://thumbs.reddit.com/t5_2s10b_6.png
* a handful in the RES Nightmode `redditDark` CSS
find_first_not_less
pull -> retrieve
resource integers -> Android resource IDs
various R class structures -> various Android R classes
ServletHttpRequest would be better, yes. I'll add this change in the new pull request.
can you remove ``--type=send.some.mail``
Fixed for all class members.
That implies different coding styles for "instance.Method(...)" and "instance.Method(...).ContinueWith(....)" calls - depending on the presence of ".ContinueWith" we would have to put the first call either on a single line or on two lines.
No there isn't any need to give a color in the input json. getNewColor is called only to assign a same color for the scatter-plot and the best-fit line dataset. Also it will give another color for next set of scatter-plot and best-fit line dataset from the same activity.
Hi Gunnar, could you please explain this comment a little? I am not very clear about it.
Isn't this comment misleading, isn't this the "region unspefic" locale?