labofsahil/github-event-dataset-2012 huggingface dataset:
As medium term a suggestion, I wonder if this option business couldn't be better mapped to Enums?

I think it provides a more typesafe option for all these Strings, and creates cleaner code allowing to simply switch on them (true, in Java 7 you can switch on Strings too, but it's less typesafe)
I can't think of a case where I've seen a by-reference success boolean either, though. In fact, I think the most idiomatic thing would be to return the `BOOL` and then offer a `GTBranch` by reference.
don't know if this is the case but can't we use something like http://pytest.org/2.2.4/funcargs.html#_pytest.python.FuncargRequest.cached_setup
here?
I never used it but from the docs it looks like it would fit in our tests
I think this needs to move to the interface used by `postReply`, where the attachments are sent directly to the `postReply` call rather than separately and manually to the `saveAttachment` method
It's a little wierd that this happens in the initialize method. Would it be possible to do it down in run. Just a conditional at the top that does something like:

if options[:console]
  run_console
  return
end

Would also mean we can remove the Kernel.exit, I think.
You can put this change into context by reviewing the commit: https://github.com/rightscale/rightimage/commit/9ded8fc87a0a12870f09c4676d3444fec383f666

It is being skipped at a higher level now.  There's no need for anyone to run this in a rebundle condition.  This not_if was added when RHEL was only a rebundle, but now we build from scratch for non-EC2.
No blank spaces must be present after opening and before closing parenthesis, like in:
(repositoryClass="Newscoop\Entity\Repository\RatingRepository")
... and make it consistent for all entity attributes
Vsechno jinak. Oznacme C jako count a L jako last_relative_time a carkou doplnek.

Kdyz timer pretece, tak kolik ubehlo? C + (L' + 1) ... tj, C plus tolik, kolik chybelo do nuly (protoze L' + L + 1 = MAX + 1 = 0)
No a to je C - L, protoze kdyz se odcita pres doplnek, tak se C - L = C + (L' + 1).

Takze if neni potreba vubec.
Use 1 or true in both places?
Needs a file header like other test framework files
This batch approach is suboptimal. See https://github.com/phpbb/phpbb3/blob/develop-olympus/phpBB/install/database_update.php#L1874 for how to do it with a constant amount of memory instead of loading everything into arrays.
Let's version the state file, so we can upgrade the format later.
Please put spaces around `=`, e.g.:
`char buf[150] = {0}`
yeah, because it suggests to use ``array`` when it is already used.
Please could you re-instate this file and add anything you require from your version
Please don't commit further to this PR!

Let's sort out what we've got here first.
On Nov 10, 2012 6:52 PM, "George Petsagourakis" <notifications@github.com>
wrote:

> In tests/Phpmig/Test/Migration/MigrationTest.php:
>
> > @@ -1,55 +0,0 @@
> > -<?php
>
> This was gone while trying to setup how the testing procedure run, my bad.
> Next commit will be about testing and will reinstate a rewrite of this
> without the Mockery dependency using PHPUnit mock objects mechanism.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/davedevelopment/phpmig/pull/23/files#r2091525>.
>
>
I wouldn't rely on this behavior - that's not a guarantee, even if it is the behavior today. 

(a) we should rescue it, (b) we should have error reporting.
Delete doesn't return a response body.
Would you please revert this and re-push? We maintain the version during cookbook release to the community site.
it's unfortunate that something as simple as dirname doesn't work properly on other platforms haha, same with `echo`
Indent code here same as elsewhere.
Be verbose. Use long names.
No, I have set a nickname in my account profile, but it still doesn't return it.
Ah, I see now in this comment which part you were referring to. I think my model is similiar, I just connected the objects a bit differently. But, from your example, I can see what you meant `Panel` to really be. I've renamed the classes to what they are here and changed the access to be more consistent, with less layers.

Probably my PHP habits to create layers because the language is not organized.
Decide if you name things `stations` or `stands`.
You have not changed to `.text` here.
Spaces inside the `{{ }}` tag.
Why this line? `mailo`? I would just remove this line.
Ok, I'll fix this. Obviously I did not understand the original code to get this.

But no problem with the omitted 'uniq'? Not here and there was one more place where I dumped it. Was not completely sure...
This import needs to be fixed, as file:./ is not going to be stable.
Setting is already persisted, so the extra 'persist' call is not necessary. Just as long the change tracking policy on the Setting entity isn't changed anyway...
This could use some elaboration of the expected XML and HTTP entity, maybe? You can infer it from the test, but it's not very clear if you're not already intimately familiar with refreshbooks' request-concocting magic.
Actually referring to the very beginning of this chapter:

> Infinispan is an open source in-memory data grid focusing on high performance. As a data grid, you can deploy it on multiple servers - referred to as nodes - and connect to it as it was a single storage engine:

Infinispan is an open source in-memory data grid focusing on high performance. As a data grid, you can deploy it on multiple servers - referred to as nodes - and connect to it as if it were a single storage engine:
No need to mention that
bad link btw
I think it would be better to try to resolve *all* hooks when circus starts and reads the config, so we raise an error if one hook could not be resolved -- instead of having an error when the hook is called
you can do that with float(stream_conf.get('refresh_time', 0.3))
PHP question: can these consts be made public? Then people who care can go Mode.AUTO (or Mode::AUTO or \Mode:+%AUTO or whatever the heck the PHP syntax is) and be assured they've got the right string.
This commit undoes some contributions from a previous ticket and commit (COOK-1194 and 6297f10), and we'd like to preserve those.
Let me go read the open issues from those referenced tickets/commits.  I'll pull them in, and continue hacking on it.
The server context is allocated in each work process, is that right?
Couldn't find anything, though I'm sure I've seen material online. When will Google add semantic analysis to search queries?
I would suggest moving the retain-autorelease dance to +taskWithIdentifier: so others can benefit ;-).
Could this be simplified to just `[entryContents count]` since 0 == nil ?
> recommended: run make html to build the docs and check errors
>
> —
>
Just to be clear, you are referring to Sphinx here right?
This is a bit messy. Clean?
Yes, that case works fine. I just tried it to verify. We get the initial list of test cases using dir() on the test class on the line immediately before the ones I removed. 'test' shows up in dir(B), so the test is run.
you should be able to use `this.body` here, as we set it as part of setting up `mulberry.app.UI`.
Same idea here.  Line 89 is a constant value but it's not obvious.
I think all these `@returns` should be `@brief Returns...`. This might fix the doxy problems Rachel saw when the brief description was an `@returns` line.
Update: I checked this more carefully, and actually, here's what's going on:
* in shear.py, calling it Shear or galsim.shear.Shear works (i.e., either of those becomes a link to the page describing the Shear class)
* in any other .py, it has to be galsim.shear.Shear
remove "marked as" since that's more of an implementation detail.
ok.  I think this is okay, though i'm would think that something like
```
{% for child in children %}
...
{% include "exams/ready/exam_child_list_item.html with item=child %} 
```
will ultimately be more maintainable.
Because javascript defaults to a cast to strings when operating on strings, so selector.value + 1, with a selector.value of 'foo' gives you 'foo1'. The *1 casts selector.value to an integer.
It's how you guys originally had it - it was too late for me to come up with something else...
CharSequence, rather than String?
`if any(app in ('django.contrib.staticfiles', 'staticfiles',) for app in settings.INSTALLED_APPS):`
On Tue, 18 Sep 2012, Hannes Magnusson wrote:

> > +		add_assoc_long(server, "port", port);
> > +		if (database) {
> > +			add_assoc_string(server, "database", database, 1);
> > +		}
> > +		if (username) {
> > +			add_assoc_string(server, "username", username, 1);
> > +		}
> > +		add_assoc_long(server, "pid", pid);
> > +
> > +		/* Grab connection info */
> > +		add_assoc_long(connection, "last_ping", ptr->connection->last_ping);
> > +		add_assoc_long(connection, "ping_master", ptr->connection->last_is_master);
> > +		add_assoc_long(connection, "ping", ptr->connection->ping_ms);
> > +		add_assoc_long(connection, "connection_type", ptr->connection->connection_type);
> > +		add_assoc_long(connection, "max_bson_size", ptr->connection->max_bson_size);
> > +		add_assoc_long(connection, "tag_count", ptr->connection->tag_count);
> 
> Hmh. I forgot about the mongos case.. we'd want to mention that 
> explicitly somehow.

Yes, we should do that. But, the "connection_type" already covers that, 
and it's even more apparent if we add the string variant of it like I 
suggested.

> The replicaset name isn't stored on the connection so getting that 
> information isn't trivial, but I think it is worth including it

I don't think we should do that as it gives the false pretext that a 
replicaset definition is linked to a connection. And they are not.

cheers,
Derick
There is an `isString` you can use.
Why null and not ""?
Why not default touch_filter to something small, like 25 or 30?
```javascript
document.body.addEventListener('load', function () {
    EventController.init();
}, false);
```
looks like this is existing code but the ```merge``` call has no side-effects and its result is not assigned to anything, making this line seemingly pointless.
Yeah, Alex has a point I think,  it might be a good idea not to add these methods at this level.  Might be best for galaxy-client to match the values in the v2 responses tightly.  If a consumer web provides an alias for one, that's fine, but probably none of galaxy-client's business.
this will actually error during compile if you try to alias it as num_purchased is undefined.
А зачем здесь new String()?
`escape` comes from the EJS utils at https://github.com/visionmedia/ejs/blob/master/lib/utils.js#L16 and it's how `<%= %>` is implemented, as per https://github.com/visionmedia/ejs/blob/master/lib/ejs.js#L129

Provider is defined statically in the `domainInfo` object in `routes.js` (https://github.com/callahad/browserid-bigtent/blob/0cf1a69e3f8fd218087150091bd936911e508df7/server/routes.js#L207-211) -- do we still need to escape it?
Sure, sounds good. I think uiIndex2 is ready to be merged, I've followed up on all of the issues mentioned in the review.
yeah, something like this makes all tests pass:

```diff
diff --git a/lib/simple_form/wrappers/many.rb b/lib/simple_form/wrappers/many.rb
index dc90a38..432706a 100644
--- a/lib/simple_form/wrappers/many.rb
+++ b/lib/simple_form/wrappers/many.rb
@@ -56,7 +56,13 @@ module SimpleForm
         return content unless tag
 
         klass = html_classes(input, options)
-        opts  = options[:"#{namespace}_html"] || {}
+
+        opts = if input_or_label?(namespace)
+          {}
+        else
+          options[:"#{namespace}_html"] || {}
+        end
+
         opts[:class] = (klass << opts[:class]).join(' ').strip unless klass.empty?
         input.template.content_tag(tag, content, opts)
       end
@@ -64,6 +70,10 @@ module SimpleForm
       def html_classes(input, options)
         @defaults[:class].dup
       end
+
+      def input_or_label?(namespace)
+        [:label, :input].include?(namespace)
+      end
     end
   end
 end
```
Trailing spaces here.
This is also one level too deep but it's elegant this way.
+1, I was about to do the same remark.
Could you please insert the first 2 plots of the example here?
Be specific instead of "those".
Someone might skip ahead to this section. Maybe "the johnson-lindenstrauss bounds" or "the bounds above"?
+1 also `symb` should better be renamed to an explicit name such as `symbols`.
composer.lock я добавлю в .gitignore, он все-равно у всех будет разный
You mean like

```ruby
```
Again, getLogs handles backward compatibility, so you use and then you can common some of this work up.
Yes, it should. You'd get away with a const char * most of the time though :)

I will fix
I don't have the rows... the rows were streamed out, key only kept. There are the only record of the fact that the rows were previously dedupped, and that all further matches should be dedupped.
Possibly - there is code like that in other places (e.g., optimizedTransformChildren) but it is highly likely that same will be false at the end of the function, and it makes the normal case more complicated.
Replace with an if statement that demonstrates that you know what you expect.
why is the last octet of the IP `[1-9][-09]{0,2}`? This breaks any networks of the form `X.X.X.0/YY` (which is even in the desc!)
Fernando C is working on this and requires the logs. Will  be removed in next commits.
ah, it absolutely doesn't. :) I'll get that fixed up.
IMO we shouldn't have this part, if it's relative and `filename` is present then we use that, absolute, or otherwise your conditional above will default it to this anyway
We now have code duplication in run_tests and run. I suspect the lines in run should be removed?
Cheater! :P
I just copied tests boilerplate from another file in this repository.
Thanks for the suggestion! I've changed it on dd5008574c.
len -> *len*
serviceName, oir storageAccountName?
Ah, there's issues with the jetty:stop due to the chaining of the jetty:run after the integration load. Leaving out is fine for now, was only a handy convenience.
`path = path.join( "/" );`
Will there always be a completion time, i.e. if this comes back null / empty what happens with the parse
range check?
yep, makes total sense!
offset instead of threshold should make it clearer.
Changed. But this is no integer division as the array elements are float.
When I change this entire `if` statement to just be `true`, all of the tests still pass. This needs a test for `should_not ensure_inclusion_of(:attr).in_array`.
Might even get away with 3px here...?
this will not be updated in the DB if the Updater is not used as there is no flush otherwise.

And this is rather confusing IMO as ``auto_update=0`` won't do what I expect (because ``(Boolean) '0' === true``)
Yeap, left over from testing.
This comment is for a property that was removed.
Terrain providers should be changed to use the error reporting system I did for imagery providers.  But that can happen in the terrain branch.  This particular error won't happen except in the case of catastrophic, unrecoverable error caused by pieces of Cesium missing, anyway.
I had a timeintervalupdater that used this to modify the start, stop and step parameters of the URI. I removed that but didn't remove this. I have no problem removing it, but I'm sure it will be useful in future iterations. How easy is it to resurrect in git if I delete it?
Did we update LICENSE.md?  Or do we consider this a tool like ant, for example?
This line creates two Cartesian3 instances and throws one away; it should be broken into two calls in order to avoid the extra instance.  The below code only creates one Cartesian3.
```
        var center = this.minimum.add(this.maximum);
        this.center = center.multiplyByScalar(0.5, center);

```
What about the other tools?
it's tabs vs. spaces
Oh. Yes. This I found out the hard way. If you have a property defined in a category "@interface foo(bar)" and not in the original interface declaration or the private interface category "@interface foo()", the compiler complains if you try to synthesize in the non-category implementation.

In short, in the .h file, we have the property in "@interface TiRootViewController (unsupported_internal)" to warn module makers to not rely on this. In the .m file, we have the property in "@interface TiRootViewController ()" so to instruct the compiler to let us synthesize in "@implementation TiRootViewController"
this won't work on windows.   do we care?  Or what about a node equiv?
Perhaps you should rename initFS to initializeFilesystem.
Add a README file to the `vsc` dir stating where you got this, and which version (git commit id) this is a copy from.

All remarks made by Andy on fancylogger should be fixed in the VSC-tools repo, not here.
```python
def make_installdir(self, dontcreate=None):
```
Please move this out to lib/amazon_seller_central.rb
Aaah sorry - @kwilczynski is correct, but not as bad as he outlines. The problem is things like this:

    Puppet::Parser::Functions.function("".to_sym)
    ArgumentError: interning empty string
	from (irb):3:in `to_sym'
	from (irb):3

Since Puppet treats everything likes strings, this is pretty much the main issue. Regardless, lets validate the input Eric - adding tests to make sure its a string, not empty etc. Thanks @kwilczynski.
You don't need hoe any more, hoe creates gemspecs automagically, you are doing it manually, get rid of it.
Sure. I just pushed a new commit.
Ah, good point. I should have thought of that. Will fix.
That worked because you had lvalues on both sides. If you checked `s1 == foo()` where `foo` returned an `S`, it wouldn't compile. Sadly, `s1 == S(1)` currently works though, so the compiler still thinks that struct literals are lvalues. I thought that that had been fixed.

> And it works with int as well. So I'm saying, apart from the doc fix we should also probably fix the compiler.

It may be that the compiler needs to be fixed to disallow returning `int` from `opEquals`, but that doesn't have to be fixed for us to reasonably assume that the user of `splitter` isn't doing that any more than we have to make `splitter` work when someone writes an `opEquals` that throws when they're not equal. I think that it's perfectly reasonable for Phobos to assume that `opEquals` returns `bool`. And in many cases, even if a type were backwards enough to return `int`, it would still work anyway.
Why are you adding parens to the template instantiations? It's preferred that template being instantiating with only one argument _not_ use parens. Please remove them in all of the places that you added them.
I think you need a comma here
this is kinda icky
maybe if it was at the top of the file?

do we need type="application/dart"? how do we (and the browser) know it's dart vs js?
No - there are some nifs out there with really large values (whence the comment above). We still want to import those.
is true? *confused*
if that don't cause an infinite loop, then +1
Typo: "downloading"
should be a winmd
Can these and the setup/teardown be moved into the test suite itself, preventing need for use of global variables? Less risk of test state leaking across tests.
We could change this to `defaultValue`, since `default` is a keyword.
I'll see if I can fix it on Friday or Saturday (@alexcole, is it the case that the heuristic will be better if we take into account class hours? I assume so). However, if I can't fix it, and if no one else can, then we should update the comments, because they are deceptive.
I'd like to store the groups in the form of ``array('group' => true)``, and then simply use ``isset()`` here for performance reasons.
ahh, gotcha.  ok, fixed.
Again, I don't think we want to confuse new users with this. If you want a literal, use 'key' instead - seasoned SpEL users will know they can use a SpEL literal here.
there's an open single-quote but no close, also missing the word "must"
Isn't it what it does? It choses the alternatives based on the IF condition.
Let me know if you still want to change it.
Don't you mean...

    Assert.isTrue(!ObjectUtils.isEmpty(objectNames), "'objectNames' must contain at least one ObjectName");

(isEmpty never returns null).
Just look at the bug... oh wait there is no bug mentioned.
Use [`assertInstanceOf`](http://www.phpunit.de/manual/3.6/en/writing-tests-for-phpunit.html#writing-tests-for-phpunit.assertions.assertInstanceOf)
nitpick: mind the laziness about {}
You should faster fill the demo page. So every second or so. So that people can see changes fast. I left it running for some time and plots are still empty for me. :-(
Do you have to create those parameters manually like you do? You cannot just pass a dict to `getJSON` and let jQuery do it? Is there some special case here?
No, good catch. Originally I had included it because that's where I was gonna do the session testing, but I decided to break it out.  There's no real code in `session/base.rb` so this file isn't really needed.
Yes, missed that one
Break this into MVP widget.  Store local copy of Entity in presenter
:+1: Man! :running:
I believe Django is far from perfect, and those days, I am actually trying to use other frameworks :P
That said, when you do a library for a framework, it is better to follow the framework's conventions.
Check here if simple value or not?
```
charset="utf-8" async defer
```
!
The `reduce` doesn't not change the original text.
This branch currently has no diffs to upstream. We could point at CloudMade/Leaflet master instead.
Yup, 

I do not want to check text input boxes in the method...

The boxes are just UI that gets the input to pass into the method.

So the HTML should read;

<input type="button" id="custom_button" value="Switch to custom URL by file URI" onclick="downloadThenBootStrapFromURL(document.getElementById('urltextbox').value), document.getElementById('usertextbox').value), document.getElementById('passtextbox').value)" />

Make sense?
We need a comment here or somewhere (top of the file in a file comment?) about how gmail basically compels us to choose HTML.
that sounds better, this is starting to make my head hurt
Great, thanks. That's the spelling we will stick with!
django-livesettings does not seem to work without real cache ...
Hmm the UserTransaction has to be on the classpath so running the code will require the javax.transaction:jta on the client side.
The evil line is about throwing errors for eval (i.e. eval is evil). It can be changed back since we are no longer linting require.js
now handled by destructor, should free everything now
Sure you could do it that way. But you'll have to include it as part of this fix.
The `cond` has no `:else` branch, so it returns `nil` for non-collections.
pythonic question...since `self.params` is always used as a dict, should it default to `self.params = {}`?
An empty dict isn't valid so the truth test works and that `else` block will always assign something.

Note: What you have works. No quibble there, just a 'teachable moment' question.
Je ne suis pas sûr que de passer de la "qualité du code" à "un code de qualité" ne soit pas un glissement sémantique. 
Dans le deuxième cas, on peut l'entendre dans le sens général, mais aussi dans le cas d'un (bout de) code précis, moins dans la première formulation il me semble.