labofsahil/github-event-dataset-2012 huggingface dataset:
Oops, I'm still getting the hang of how the workflow works here.  I've already pushed some more changes I wanted to make, before responding to this review.  I guess I should have done that in a branch or something?  But I seem to only be able to have one pull request active at a time.  You'll have to tell me how this is all supposed to work, tomorrow.  In the meantime, I'll just combine all the changes into one, including your suggested fixes, and update the pull request.  You may need to look everything over again from scratch. :-(

Anyway, made this fix, which is clearly the right thing.
If we would two options parser, it will not help us, since we have to check what exact parser need to be parsed.
I've separated options into the common and run/deploy options to have them separately in help message. Also created usage messages for both 'build' and 'run' commands and issue them in certain cases.
endpoint + add a message to checkNotNull like "endpoint is null"
I think this should be #ifdef'd for Solaris. Otherwise we're allocating extra memory on non-Solaris platforms.
Who uses KVO anymore? :trollface:
The index=0/index++ scheme works well, but if you want to be slightly more elegant/readable, you could replace it with:
```
for index,pluralRules in enumerate(doc.getElementsByTagName("pluralRules"):
```

which would do the same thing but avoid the risk of the index being corrupted if some day you add a `break` or `continue` that bypasses the trailing +=1
Now , the default config.yml exist , and we need to load the custom config , also no need to copyDefaults options, just load that
you should add a use statement, and you should use dependency injection for the Buzz browser to allow configuring it differently (it will make your tests easier btw as you will be able to mock it)
(maybe suggesting a minimum value in the README would be good - at least `60.seconds`?)
In camel case, the name of the variable should be connectionTimeoutMillisecond
Pluralize newEmails - and getEmailMessages
It is very specific to this testcase so I don't think that we are using it anywhere.
@christianv Right you are - having the full URL would def. simplify things here.
Why are you iterating here?
I tried to avoid alignment problems, but you're right :) updating
I more interested in you tell me which method is the more generally accepted with MSF modules.  Is it better to just handle exceptions within the main run method?  Or should I handle within each other method.  If so do you know of an example module where exceptions are re-raised?  Please offer your personal opinion as to what is the right course of action.
Seems I forgot the docstrings. Yeah, I guess it's better to remove them if they don't contain any information.
", passed as-is to ndimage" -> "(See `scipy.ndimage.map_coordinates` for details.)"
I'd prefer this to be called `create_instance_in_database` or something - it's not setting up an existing item, it's only creating one. Also, I'd do this:

```ruby
@existing = create_instance_in_database
```

So that you know where the variable is being set.
What kind of markup does this deal with?
I'm pretty sure this is the precision problems we are seeing in Columbus view.
Where did you get this list of image extensions?  Is this all formats supported by the JavaScript `Image` object?  CC @shunter
We could do that for sure, just thought that this would always return the same default Route so we could get away it only creating it once.
Same as CoverFlowView.
yay astwalker!
Maybe like this? 

```php
    $urls = $rows->each(
        function($node) {
            if ($node->getElementsByTagName('td')->length <= 0) {
                return;
            }
    
            return $node->getElementsByTagName('td')->item(0)->textContent;
        });
```
at least @boegel  should be acked for the restructuring and testing etc etc
it shouldn't be compiled if not WITH_ZEITGEIST

it should be something like

if WITH_ZEITGEIST
  dawati_panel_applications_SOURCES +=  mnb-launcher-zg-utils.c mnb-launcher-zg-utils.h
endif

instead
> How on earth are you getting a fully immutable array in a template? Do you have a concrete example of this or you simply assuming that this is a problem based on your reading of the code?

I traced it via simple (static) printing: `pragama(msg, Range.stringof);`: It's definitely there.

> Implicit Function Template Instantiation. It's what happens when you use a templated function without explicitly instantiating it. The template arguments are inferred from the function arguments.

Ah I see... But that stops once varargs get int the story (apparently). This passes:

```D
auto foo(Args...)(Args args)
{
    static assert(is(Args[0] == immutable(int[])));
}

void main()
{
    immutable(int[]) a = [1, 2, 3];
    foo(a);
}
```

Is this a bug, or do varargs explicitly bypass IFTI?

Where I'm calling `lockstep(args...)`, which is taking it's arguments as immutable, then forwarding to: `Lockstep!(Args...)(Args args)`, which is passing the array as explicitly immutable. I guess that's how it happened.

As I said, there was probably already something strange going on there, because *I* didn't insert the first unqual. This is in the function signature:

`allSatisfy!(isInputRange, staticMap!(Unqual, Args[0..$ - 1]))`

BTW, I also noticed that a foreach can iterate on a static array. Should this code be made legal?

```D
void main()
{
    int[3] a = [1, 2, 3];
    int[3] b = [4, 5, 6];

    foreach(ref u, ref v; lockstep(a, b))
        writefln("%s & %s", u, v);
}
```

In any case, I'll try and see if I can't do some work inside `lockstep`, to fix the issue at the root, rather than band-aid the implementation.
The end result is the same.
Haystack is saved before call to find and thus haystack.length is the same value later on (and fetched in the same way).

How about just leave it as is, because at the very least the comment is misleading (as haystack *is* saved).
Merge into the previous line?
Given that `allSatisfy` takes a template - it uses `F!(T[0])` / I don't see how you can expect it to work with a function? We _could_ make all of the templates like `allSatisfy` take functions (I did initially with `Filter`), but it's a loss less code duplication to just create a wrapper template for functions.

And do you mean that you have difficulty figuring out some good use cases for `Lambda` or for operating on a `TypeTuple` of values with a function? Given than `TypeTuple` is how you get static `foreach` to work, having the ability to operate on `TypeTuple`s of values can be quite useful.
Doesn't matter; it's part of the C API, so it should be public.
Ditto. Let's not adopt a blank-minded style convention please.
here again the curly bracket needs to be on the next line .. also do not omit the ``public`` keyword (applies to the following methods too). finally add some white space between property and method definitions.
these brackets are a little weird - we tend not to do that in the OF examples or codebase.
I didn't want to wait for the update and its not a huge one as we need to go through PG and do a lot of cleanup still. 

T
Instead of calling this again, can't we just set up a variable with permission when we initially check?
These two methods centralize the isAdmin check. isAdmin() checks the current user. isAdmin(String) will check any given user ID and will throw AccessDeniedException if the current user is not privileged enough to check the status of the supplied user ID.
Hmmmm! Xcode up to its old tricks again?!
If you're going to use a ```forEach``` on this, you need to check that it is an ```Array``` and if not, handle the error.
Should be
```php
foreach ($cats as $c) {
```
like above: basically make sure that all actors have nice names.
na na
aslan rahesh in nist !
I would also allow numbers here. For example, jQuery sends this callback "jQuery171065827149929257_1343950463342".
`path` and `filter` exist to restrict the list of tests to run.
Indeed. Facts (and related Cache derived) classes are probably good candidates to be mocked in the test fixture. Since pretty much anything except for maybe test_facts really wants the mocked versions.
Fugly, but I see why it's like that.
El wrapper conoce al parser, en todo caso si efectivamente necesitas que el asiento que se crea conozca al wrapper, lo que deberías hacer es:

#AerolineaLanchitaWrapper
public List<Asiento> buscarAsientos(Vuelo vuelo){
String[][] asientosLanchita=AerolineaLanchita.getInstance().asientosDisponibles(....);
return this.parser.parseDisponibles(asientosLanchita,vuelo ,this)

y agregale el parámetro a parseDisponibles, para después usarlo cuando llamás a create()... tendrías que agregarle un parámetro más a create, pero así es infinito mas prolijo.
also, the tick marks get lost in the markdown

tick tick space tick blahblahblah tick space tick tick

`` `like this` ``?
It can stay for now. There's a backend ticket dealing with search on type (jpeg, pdf,...) and once that's finished we'll uncomment and hook this up properly.
the only situation in which the rollback needs to be send around is when the prepare was send around. This is already tracked in the ReplicationInterceptor.visitPrepareCommand:
 ((LocalTxInvocationContext) ctx).remoteLocksAcquired(rpcManager.getTransport().getMembers());

then the same interceptor, in visitCommitCommand, won't send the commit to other peers if the prepare was not send:
if (shouldInvokeRemoteTxCommand(ctx)) --> 
   -- shouldInvokeRemoteTxCommand does the following: 
      return ctx.isOriginLocal() && (ctx.hasModifications()  ||
                                           !((LocalTxInvocationContext) ctx).getRemoteLocksAcquired().isEmpty());

So this scenario seems to be tracked already by the code but an UnitTest should tell this for sure. If I'm not mistaken then the check should be dropped together with the shouldInvokedRemotely flag in the PrepareCommand.
is *thrown*
No, we can't do that since every time you set http-method-expression you'll get mutual exclusivity
Should be 
``CREATE_MESSAGE_GROUP```
``UPDATE_MESSAGE_GROUP```
The names of constants don't reflect their life
Actually we do want to keep that. I understand that each manual is specific to the version, however since by default the manual displayed is the most current and the framework used might not be it helps to quickly point misalignments.
However, once we go 3.0 (a new major release) the manual would have to be changed to remove all references to 2.x and simply show something as a regular feature (not new)
hmm probably not. let's just leave it unpinned for now, when bs4 gets its first major release we'll pin it.
We have a very similar function in WP if you wanted to have a look it is under the webplatform/framework/lib/utils.js file.
OK, good to know.  I'm good with this change.
probably the private was fine here
whether to throw exceptions on untranslated child documents (not nodes, translation is a pure odm concept that phpcr does not have) or to not include them
I dont like how you prhased "blag"
Btw., by googling how others address that problem I stumbled upon [this post](http://java.jiderhamn.se/2012/01/01/classloader-leaks-ii-find-and-work-around-unwanted-references/) which discusses our very example :)
This was already available as `system-upgrade` which I've tweaked in e8799323f72d9a57549e18cb59988d3f00317b2e. Be careful as it's known to break VirtualBox Guest Additions when a new kernel is installed and the DKMS module is not re-compiled. [dotless-de/vagrant-vbguest](https://github.com/dotless-de/vagrant-vbguest) is good at handling this case.
you should really be checking the data coming back.
Missing space after comma.
Also, break this call into multiple lines.
stackIndex would be clearer :)
Yeah, that's unnecessary, I'll remove it.
It's slightly inefficient to package it in Integers with each call. compareTo will be called a lot (> few thousands times for a package). Either change private fields to Integer or use ints here. I think a fix with ints would be much quicker. See above.
Deafult > Default
What do you think about adding manager name into the @Transactional annotation?
use limit offset for api consistency
Vaihtoehtoisesti (<= 13 age 19) :)
The string values for contribs and collabs is controlled here, in the corpusmanagement file, as well as in the FieldDB public/corpus/Corpus.js and public/permission/Permission.js