labofsahil/github-event-dataset-2012 huggingface dataset:
Changed to:
```javascript
if (layer instanceof OpenLayers.Layer.WMS && (layer.url == "/geoserver/wms" ||
                   layer.url.indexOf(this.localGeoServerBaseUrl.replace(this.urlPortRegEx, "$1/")) == 0))
```

- Check to see if it is a WMS layer first, otherwise it might not have a url
- If local WMS layer is already part of the map, it will have URL of "/geoserver/wms", but if added from 'Add Layers' dialog, it will have a URL = settings.GEOSERVER_URL
this comment sucks
I see now. I thought the Profiling came from the gem but see it's a module. Looks good to me.
We could do `null != charset` here.
Entry point providing an XmlDocument
So I just add a default value ?
This is wrong.
https://help.github.com/articles/ignoring-files
[Logstash](http://logstash.net/) -> [logstash](http://logstash.net/)
How do you get dupes and are you eliminating duplicate instances based on obj reference?
Why do I need to check for timeout value to escape?
Seems a little strange to group `{Mod, SrcIdx, TargetIdx}` without also including `TargetNode` in that group -- technically, all 4 ids label a given transfer. Then again, it looks like there's lots of logic that just uses `ModSrcTgt`. I guess that's safe for now,  ownership/repairs are always `primary -> primary` which has the implicit guarantee that `TargetIdx` also uniquely ids `TargetNode` (the primary owner). And secondaries are `secondary -> primary` which has the same benefit. Just something that caught my eye and wanted to note. Maybe we should just document this implicit assumption somewhere.
I don't understand this change, why logfile first and in the end log file ... I thought log file is the correct spelling?
More.  "-Wno-"  deactivates a warning (e.g. for ones which are otherwise turned on by -Wextra or -Wall)
sure, I just copy and pasted the old format method here. But I can refactor accordingly
You need spaces around `.`.  This also introduces new markup for the current page which may break existing stylesheets.
boolean flag?
Indien rechten altijd toegepast zouden worden, en een gebruiker komt dan op een pagina waar hij geen rechten heeft, dan wordt hij naar een 404 gestuurd omdat die pagina zogezegd niet bestaat (volgens de navigatie), wat dus niet echt klopt.
755 does make more sense. I'll change that.

With regards to `Sys.chmod` on Windows, I'd suspect that if it doesn't work, it'll degrade nicely. It is in the base package after all. Anyone with a Window machine could test that?
I guess what it needs is a more complete docstring: regexps lingo is often ambiguous, because people often use the word 'regex' to describe either the *pattern* (which is a plain string) or the *compiled regex* (which is a very different object from a string).  So I'd like the dosctring here to unambigously and explicitly indicate what it refers to, what it validates, what the constructor takes, etc.  In other traits we don't need as much info b/c everybody has a reasonable idea of what an Int should be.  But I think regexes deserve special consideration to avoid confusion in the future.
please use LinkedHashMap for deterministic ordering in tests, etc.
else if for consistency?
I'm thinking: `GIT_EUSER`?
The javascript debugger did not like this, so I changed the URLs that reference this to use chrome://keefox/content, which points to the same place anyway. I hope this is OK.
I think we should show the real replies count now and just max the percent at 100
Looking good! A few more nitty things from our yet to be published JavaScript style guide (https://github.com/mozilla/webdev-bootcamp/compare/master...js-style-guide).

1- Always use single quotes for strings.
2- Always use `===` for comparing (never `==`)
I would move the `__status__` line into `endModule` now.
I prefer to specify a help_text field in the models that is a bit more verbose, ex: "The new password you want to use on your account", "Please verify your new password."
What is reason here? It looks like quite strange here. So you don't want to really do actions? Why? At least comment would be nice.
Short tags are fine
Check for typeAstOpr, so if other types slip through, it still works..
s/which/env ?
Thanks! dd16a91b6e8ffbddef5443207618e459daf91030
This method is misspelled - `inititalize` => `initialize`
Since it's within the same class I don't feel it's an issue. I'm used to using constants if the value changes depending on the environment, or if the value needs to be referenced from outside places.

Either way, if this is necessary I think it belongs in a separate pull request, changing it for all the DataCollectors at the same time.
In the source code I agree but does it really matter in a test? That makes it really easier...
@stof?
I mean no... but we don't care any more as there is now a single generic case
Valid concerns. (Even if we revert changes to this file, becuase RequestMatcherInterface depends on Request as well.)

The last approach wouldn't solve the original problem, that an implementation might require the full request object - *always*. Maybe that's what's wrong in the first place: Make the Drupal implementation only use the URL and the request context. Nothing else.
Could you add comments in a similar style to the other functions? And could you also add some test cases in ClientSpec.scala for the new commands?
volatiles on same line: `@volatile private[this] var ..`
moved to: 

    NSURL *url = [NSURL URLWithString:[kLinkedInAPIUrl stringByAppendingString:@"/uas/oauth/invalidateToken"]];
    OAMutableURLRequest *request = [[OAMutableURLRequest alloc] initWithURL:url
    ...
Same remark as above. Perhaps it is a better to ask for output stream and let the user open the stream with a filename if he wants to.
This will be a class that users will extend. Can we add nice docstrings to it?
it's the latest in maven central, doesn't the internal build fall back to that?
I had to do this in a previous commit where I didn't have ```format_dict``` to hand in a property. I'll fix it.
Ahh, I see. I don't s'pose this could be raised in a separate ticket?
Any particular reason why this hasn't changed to Robinson like above?
Shouldn't ```buf``` always be 0 length, since we asked for ```size - len(line)``` bytes? If it can ever return something, maybe a more descriptive name than ```buf```, like ```extra_data```?

And can ```self.reader.read()``` ever return fewer bytes than we asked for without hitting the end of the source stream? (e.g. how a socket read will hang if it has no data, but may return less than requested when it has any) That could cause this function to return an "extra" partial line.

Maybe Python protects fully against that…
I did some stuff with recompiling the protobufs, but 4ad has merged that in. I'll switch back to his import.
Use something like ``from distutils.version import LooseVersion; DJANGO_1_3 = LooseVersion(django.get_version()) < LooseVersion('1.4')``
@bestcroftington Can you get rid of this added require?
I'd probably do it in the setter. These notifications are ultimately getting fired off in the setter, anyway, and there's little reason add the verbosity of two observers, here.
Re Sensei,

Oki, C'est bon à savoir ("Tu peux sortir le Pattern et le mettre en static, il est threadsafe. C'est le matcher qui n'est pas threadsafe."), 

Et effectivement il sera plus pertinent de mettre login que user!

F.
After deploying a JPA application, it took about 80ms to shutdown the AS instance with a Ctrl-C or "./jboss-cli.sh --connect command=:shutdown".  I did the shutdown within a few seconds of deploying the test JPA app.

I do agree, that the pool will stay open as long as the PersistenceUnitServiceImpl keeps its static reference to the pool.  But, with my (after JPA deployment) testing, there doesn't seem to be a 60 second shutdown delay caused by that.
:(

What a bad message to have gotten wrong! (This and the next couple).
I think we need to fix the real issue here, rather than changing this to a Set wab deployments should not add the deployment root to the resource roots list, it will break other subsystems that use resource roots, for instance this will cause weld to create two different bean archives for the same deployment root.
With Java couldn't you write the 8-byte long into a temporary byte array (you can specify an ENDIAN too so you know what order the bytes are in), then just take the lower 4 bytes and put them in the data output stream?
We used to require anchors in other places as well, I think old Safaris sucked at handling tabindex. That's why Accordion doesn't require anchors anymore, that was one of the big changes in 1.9. We should try to replace the anchor and see if that helps improving screenreader compat anywhere.
If you're going to cache these, you should do it at the very top of the file so it only happens once.
All calls to `getSwarmUAIndex` occur when it is set, except for one. The `Validate browserSets` section in `init.php`. I suggest you either move that section to below the assignment of `$swarmContext` or, probably better, take a good look at that section and refactor it to not use `getSwarmUAIndex` in the first place since it looks like it won't need it anymore in the new system.
`element !== fabric.document`
Quick question, instead of having a key that says if we have backward, is not better to check the step and don't allow to go back if you are in currentStep 1?
Yes XD
Is not the button in self, just the `label` tag that appears together, and in that way is how it should look like the examples page.
Ok! Thanks for the tip!
need to update this function for composite key without keyCode.
you dont need to escape quotes
ben> I think the implicit agreement is to not keep dead code, so I'd rather remove everything dealing with this `withAnimation` variable. There is no need for a refactoring, just removing lots of parameters and `if` code.
Why a switch here? I would use the same code but removing or adding the class depending on the event.target.dataset.action
line break after comma
Agree, single way for all the activities.
indeed it is, but iss is a standard JWT field that I doubt we will change soon. If we do, then yes this would have to change.
Elsewhere, we extract an origin from a URL via:

    url.toString().substring(0, url.lastIndexOf(url.path));

Shouldn't we do the same here?
If you do keep this as a private helper method, it'd be nice to use the `remaining_unused_interaction_count` method rather than `@interactions` directly--when possible, it's nice to use the pubic API even internally.
Oh, you think so? Ok.
Can we make the YEAR_MONTH_DAY pattern an implicit so that it's more easily configurable, without subclassing/modifying all N of these classes?
No need for request.render(). It will also not get cleaned once reply is done.
please change to:

    __rvm_version_compare "${rvm_ruby_version}" -lt "1.8.5"

this will ensure version is compared as version not as string
Maybe make this a property on the app, seeing as it is state?
You're missing something here.
Typo fixed and added a todo(mdw) for the unit test (you can reassign this later).

I thing the this method is a closer fit to the other conversion methods in helpers/util.py as things are right now.  The only things in model.py are the actual model classes.  I think that if the number of conversion methods grows, then it might make sense to have a model_util.py.
I do, but this is copied from the crappy CSS for the main Affiliates site that needs to be revamped and LESSified. Will add the space in.
Wouldn't

"Gets the ChatSections' List for this request" 

be better?
Why is this necessary?

You might also want to consider doing this by making ticket the instance variable and then asking it for @ticket.errors in the view, but that's mainly a stylistic point
should be an absolute import
Actually, describe returns a list, so it would probably make more sense to just build a list, rather than having a space here, and then splitting later.
I don't know either, I just thought I'd mention it :)
If you take a look at pinning/internal/header.m4 you could either simply add 

#if (USE_USART == 0 && defined(HAVE_RS485TE_USART0))
 #define RS485_DISABLE_TX    PIN_CLEAR(RS485TE_USART0);  // disable RS485 transmitter for usart 0
 #define RS485_ENABLE_TX   ......
#elif (USE_USART == 1  && defined(HAVE_RS485TE_USART1))
 #define RS485_DISABLE_TX    PIN_CLEAR(RS485TE_USART1);  // disable RS485 transmitter for usart 1
 #define RS485_ENABLE_TX   ......
#else
 #warning no RS485 transmit enable pin for ELTAKOMS defined
#endif

and use RS485_DISABLE_TX at the original code. This only moves the proprocessor defines to pinning.c, which is included by many files.

The other way would be to let m4 produce the USART0/1 defines.  The following is untested:

#define RS485TE_USART format(RS485TE_USART%d, uart())
typo: receiver
Да, все ок
There were both kind of .ready() handler syntaxes before my changes. It was weird. I've left more readable.
Isn't it more readable?
Perhaps a better way to handle this `udp_sender.c` code is to only have the `udp_client.c` file, and just pass in another command line argument which is if we want it to be one way, or not.  What do you think?  So really the only difference compared to the previous code is that 

```
if (argc == 6) {
				do_recv_proc(fdr, msg_size);
			}
			foo++;
```
All of this stuff requires a comment to say "don't delete the memory until all cores exit" and "make sure that the initializing core does the initialization before any other core progresses".
Another issue with loading it after bootstrap inside the module is we will have a FOUC, but this may be ok as the tweets just like like block-quotes without JS enabled. ???
I've added `language` attribute to core.
It looks as the best matching name for such attribute:
  http://tools.ietf.org/html/rfc4646#page-5
But we can use `shortLocale` instead?

-or-

Rename `locale` to `language` and add `shortLanguage` in order to better match `navigator.language` (which is equal to current `locale` attribute)
'handle' doesn't seem to be a very explicit name for what's happening here, but that's not part of this patch, so can change or not as you like.
Please call either `onTabUnload` or `tabUnloadHandler`.
Ah, ok. Can we call it something like "lazySetImplicitWaitTimeout" so it's clearly something different? I'm afraid I would not have caught this if skimming in a hurry.
I like that this logic has been moved to a model
Agreed, this is a bit of carry over from an early abandoned strategy.  I will work on refactoring adapt_data and the associated adapter methods.
Can you explain what line 16 does, <code>self.is_the_current_page</code>. I'm still unclear what it accomplishes since that variable is set in the <code>BasePage</code> class and in its usage here we're not doing anything with this variable. It's not being used as a getter or a setter.
Make sure to report this change in dynohost/bin/dynohost
As currently written, this isn't a class, it is a collection of global state.  If it stays in that form it should be changed to a module to make it obvious that this can't, and shouldn't, be instantiated.  (It probably should also have initialize made private to help enforce that.)

That said, it shouldn't do that.  We have at least one mode where Ruby level threads are used to provide services to clients, the Webrick based master.  In that mode we can have multiple concurrent compilations happening, which makes this global state ... troublesome: if two clients compile concurrently and use Ruby code they will walk all over each other randomly.

This needs to become a real class, with one (or more) instances per thread of operation, probably driven by the compiler.
I think this is misleading: this doesn't validate arguments, but it does delegate to the function which, ultimately, will validate them, right?
-e ricarica-
sorry, will fix that
By convenion, all dependencies need to be imported explictly with a "use" statement
Why not just "php_sapi_name() == 'cli'" ? Why don't use "Console::isConsole()"
Is using Reflection here not jumping out as a bad idea?  Would you expect that by calling this seemingly innocuous method (SubClass::isSubclassOf()) would go and instantiate a reflection object to answer the question?
Also, instead of implementsInterface, I think you were looking for $r->isSubclassOf().
$schema isn't used in this method. No need for else in order to assign to an empty string.
This is quite problematic, then. Re-adding the silence operator removes critical error-reporting capabilities (and in turn raises errors when using with xdebug or the scream extension); but, as you note, makes this difficult if not impossible to use inside another error handler. 

I think it may make more sense to add a switch to the writer, "isInErrorHandler" or similar, that, when enabled, switches to error suppression. This would allow the Stream writer to function under normal usage and provide error reporting, but also be used inside an error handler safely.

Thoughts?
Does this still build under the NET20 profile in the solution?
This should be just `SolrIndexer`.
Good catch! Yes, it will just pick it up and work.
All this msgqueue stuff make me really worry. Why is this necessary?
This is supposed to be the age of the world IIRC
Can be: `<li class=" {{#if todo_completed}}completed{{/if}} {{#if todo_editing}}editing{{/if}}"></li>`
Where and how are handled multi-part messages ?
Unless I miss something this only works correctly for one part messages
This return is useless here. You can just remove it.
Y NO SIMPLE FORM?
better to use self.__class__ here instead of EB_ParMETIS
shorten this line, e.g. introduce "srcdir" to make it more clear
obj.is_a?(Issue)
Crazy-nit, but single quotes over double. ^_^
тут было бы хорошо добавить пару слов на тему что кастомные — это не дефолтные, и дать ссылку на список дефолтных полей
The `res_list` may not have elements in it if the bootstrap service is not started.  Bootstrap is not a requirement in the service clients list therefore we can't make assumptions about the existence of resources it creates.  If bootstrap service was not launched and line 64 is executed the container will crash due to a `IndexError: list index out of range`.
Sure.  I don't particularly see the behavior itself as problematic (I'm not sure whether people will be working with both python- tryfer and node-tryfer, but you might want to make a note that the behavior is different, in case they're expecting it to be the same).

I think `maxIdleTime` might be a confusing option name though because it doesn't really seem like an upper bound - the idle time could be happen longer, depending on circumstances.  Maybe just `sendInterval` or something like?
Actually that was unintentional, but now that you mention it I do prefer it. I'll update the comments and tests to match.
Why `Long`?
Doesn't all icinga server installations use php4nagios? Maybe remove the "syco install-pnp4nagios" alternative, and call the function directly from _install_icinga(args) ??
NOT a big deal but:

maybe this is better done in JS instead, e.g. if you have a radio button and one select (that does the show & enable or hide & disable).

its a UI issue, but seeing the two dropdowns makes the form a bit more complicated, esp when you have  many interfaces.
In the connector, this is executed before ```_itemType``` is actually set, because ```getTranslatorObject()``` operates asynchronously. In theory this should work if you call ```Zotero.wait()``` here and then call ```Zotero.done(_itemType)``` from the callback passed to ```getTranslatorObject()``` once _itemType is set.
Hmm I've been looking into this, and I think since listdatastreams is use to populate the list of datastreams for the object it will still list datastreams that have getdissemination denied, so I think we can leave the access denied checks for access callbacks or menu callbacks to handle.
Ohhhhh yeah I forgot that's in Mercurial :)
additional desc for "list"
Could you also remove the underscore key from the devDependencies list?
Should the Wikipedia link link to the phpBB page?
Those who build multi lingual websites would very much like to have this to make sure you can actually see the translated block name.
At the very least, mark it clearly as a hack, with a todo which is clear about why it was done and when to remove it.  When hacks are marked they can be scheduled for removal.
Я предлагаю все-таки оставить параметр командной строки "directory", а тут title альбома использовать по-умолчанию.
Does all this logging actually work? I do not see any default logging configuration.
Thanks for bringing that up. I haven't used Objective-C ARC, so I don't have great context on the subject, but after reading [this](http://www.rubymotion.com/developer-center/guides/runtime/#_objective_c_and_corefoundation_objects), I was uncertain about whether retain/release really had any effect on Ruby objects in RM, or objects in RM at all.
Sorry for not answering before, How about you send an email to the Lift
mailing list, asking for feedback, maybe there people will say to just go
ahead with your changes, or they may have some concerns (because as it
stands, I'm just blocking your pull request but I'm not giving you any
solutions, which I don;t like.

Thanks

  Diego

On Wed, Oct 3, 2012 at 6:29 AM, Tobias Pfeiffer <notifications@github.com>wrote:

> In src/test/scala/net/liftmodules/textile/TextileSpec.scala:
>
> > @@ -40,20 +40,20 @@ class TextileSpec extends Specification {
> >      "deal with multi-line div" in {
> >        val div =
> >  """<div class="vcard">
> > -   <div class="fn">Joe Doe</div>
> > -   <div class="org">The Example Company</div>
> > -   <div class="tel">604-555-1234</div>
> > -   http://example.com/
> > - </div>"""
>
> Well, one of the main motivations for my changes was to change the
> TextileParser's behavior for this test. Currently (at 53e190f<https://github.com/liftmodules/textile/commit/53e190f35574c7bd09e03102bff406fa522566e0>),
> the string
>
> """<div class="vcard">
>    <div class="fn">Joe Doe</div>
>    <div class="org">The Example Company</div>
>    <div class="tel">604-555-1234</div>
>    http://example.com/
>  </div>"""
>
> is transformed into the XML:
>
> <p><div>
>    <div>Joe Doe</div>
>    <div>The Example Company</div>
>    <div>604-555-1234</div>
>    <a href="http://example.com/">http://example.com/</a>
>  </div></p>
>
> I wanted a) to be able to include classes (which would break this test,
> but probably noone's existing markup) and b) to get rid of the surrounding
> <p> element (partly because nesting div into p is not allowed). In fact, I
> think that if we leave the input string unchanged (i.e., including spaces),
> the parser *should* transform it to:
>
> <div class="vcard">
>    <div class="fn"><p>Joe Doe</p></div>
>    <div class="org"><p>The Example Company</p></div>
>    <div class="tel"><p>604-555-1234</p></div>
>    <p><a href="http://example.com/">http://example.com/</a></p>
>  </div>
>
> and I can try to make it behave like this – what do you think?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/liftmodules/textile/pull/6/files#r1748487>.
>
>



-- 
Diego Medina
Lift/Scala Developer
diego@fmpwizard.com
http://www.fmpwizard.com
I'd prefer if we dealt with `Sitemap::Page`s instead of string paths wherever possible. It's much easier to know what they are, and you can ask the object questions about itself without having to go back to the Store or copy-paste stuff like `File.extname(path)`. That's why I tried to get rid of all the `Sitemap::Store` methods that returned paths in favor of ones that returned objects.
I think I commented elsewhere on your wait_for_page_to_load proposal. I don't think this is the right solution as a page loading being complete is different depending on your context.
yes this is i get after merging my fork with the latest original repo
I know about the change but that sould be done in the pull that changes all the addons....
Currently in master we have Adblock Plus
https://github.com/mozilla/Addon-Tests/blob/master/tests/desktop/test_reviews.py#L75
This isn't really the level number, it's the IFD or directory number.  Perhaps it should be renamed?
This is what pr-str gives... pretty useless.  I'll see what else I can find though.

    Query Params: #<Entry 1=File>, #<Entry 2=/etc/ipsec.conf>
An additional note on that, once that is done, I don't think `getFullConfig` should really even be asynchronous anymore. It should maintain a contract of being a quick, non-IO-bound access.
Done as part of making sure the config can just use `extend`. Fixed all tests as well.
should be for loop now
I can see why the additional guard is really useful to avoid the an old ``compiler`` instance can be leaked by the ``MarkOccurrencesIndex``.

However, why does it need to be wrapped in a ``WeakReference``? The ``ScalaOccurrencesFinder`` is a member of the ``ScalaSourceFileEditor``, and hence the instance (an the map content) should be garbage collected the moment the editor gets closed. What am I missing?
Would `prefix` be a better name?
For some reason I am not a big fan of this method. I would prefer a `resumeToSuspended()` on  the same model as `resumeToCompletion()`.
And it would replace the double call you are doing every time: `session.continue(); session.waitUntilSuspended()`
Comments here are also based on the assumption that you would move the logic that is in the Controller to here.
Why we use flush only in one case?
At a second look, with the new stuff - this one should be made an `elseif`.
Ah ... well, at least someone's paying attention to this one. Thanks. :)
Вы даже не представляете на сколько это интересно, изучающим людям,  когда в одиночку "точишь" по тихоничку, без общения, а тут такая феерия. 

Сп. большое
а если размер будет не 100, а 200 
работать не будет
I am pretty sure that the "r=public" is a magic / ness specific thing, as other examples (e.g. http://stackoverflow.com/questions/7911620/using-the-nexus-rest-api-to-get-latest-artifact-version-for-given-groupid-artfic) use different r= parameters. 

Can we omit that? What would happen then?
Sure. Forgot about them ;-)
No.  That method's signature is `def from_url(cls, url, db=None, **kwargs)`.
describeStart is angular scenario function added by Tim
Remember to always use 

``` javascript
this.$(selector)
```

instead of a global scope as it is.
update javadoc - incoming HTL of 0 is not taken to be 1
Can you please [add parentheses around this ternary expression](https://github.com/github/objective-c-conventions#expressions)?
Okay, I also fixed an unrelated typo :)
Please use the absolute path and offset it by a blank line
what I meant here is to change the name of the function not the name of the argument.
change this to set check interval
Is setting projection to null necessary? i would assume it is already null
This was my code, and it's just to autofill stuff. The user still has
control over the selection afterwards. However, it should probably be fixed
:)

On Thu, Sep 27, 2012 at 6:07 PM, Chris Dickinson
<notifications@github.com>wrote:

> In media/javascript/rtd-import.js:
>
> > +
> > +      if (val.indexOf("git") >= 0) {
> > +        repo.value = "git";
> > +      } else if (val.indexOf("bitbucket") >= 0) {
> > +        repo.value = "hg";
> > +      } else if (val.indexOf("hg") >= 0) {
> > +        repo.value = "hg";
> > +      } else if (val.indexOf("bzr") >= 0) {
> > +        repo.value = "bzr";
> > +      } else if (val.indexOf("launchpad") >= 0) {
> > +        repo.value = "bzr";
> > +      } else if (val.indexOf("trunk") >= 0) {
> > +        repo.value = "svn";
> > +      } else if (val.indexOf("svn") >= 0) {
> > +        repo.value = "svn";
> > +      }
>
> won't this catch if the repo value contains any of these characters
> anywhere in the string? like, hg://gitastic might cause RTD to think it's
> a git repo instead of a hg repo?
>
> also why unwrap the repo element?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/rtfd/readthedocs.org/pull/260/files#r1713924>.
>
>



-- 
Eric Holscher
Engineer at Urban Airship in Portland, Or
http://ericholscher.com
please wrap the line after the first crossing crossing 72 chars
This seems like a very dubious way to initialize a `Mutex`. If you have two threads that call `middleware_mutex` at the same time, they may get different mutexes the first time.

Also, instead of trying to use `@middleware_mutex.locked?` to create a poor mans recursive mutex, this should really just use `Monitor` instead. It acts just like a mutex but it's recursive.
I'd like this out of the README.  I'm all for people marking up their fork readmes, I just don't want it on the source.
Be sure to identify paid resources with a `($)` as we do in other trails.
Hi Kyle,
     Lata, Postal code and rate center are optional parameters, so I'm curious why we're not just pulling them out of the kwargs array with 

    kwargs.get('rate_center', None)

Unless this is another thing for backwards compatibility?
I don't like comments hanging around
This bit isn't needed.
Yo le sacaría las negritas.

En cuanto al testeo de la aplicación se decidió continuar con el test informal que estamos realizando actualmente y se intentará realizar más tests unitarios de las nuevas features desarrolladas. Pero no se invertirá demasiado tiempo en eso ya que, ahora mismo, le prioridad del cliente es la funcionalidad. En paralelo investigaremos [Selenium] para realizar tests de integración.
* "tests" en lugar de "test"
* "funcionalidad" en lugar de "funcionabilidad"
A local avatar of mine that I ignored.
Hi, can you remove this as this is "imported but unused".
Right, it's unnecessary
The rationale behind this was DistributedTaskLifecycle and its callbacks. For distributed executors we need a common super interface for method callbacks in DistributedTaskLifecycle which is Callable. RemoteExecutable was introduced to have only two callbacks (pre/post execute) for both Mapper and Reducer. That is it. If there is another, better way, to achieve this - I am listening! We could have seperate callbacks for Mapper and for Reducer in DistributedTaskLifecycle and get rid of RemoteExecutable?
Ok I see, I  can use TopologyAwareAddress :-)
Good question… a diff missing a hunk is still a fucked up diff.
*`NO`, `YES`
Looks like you left a print in here
Is this class a separate page or a region?
If this is a region then we should move it to a /regions/folder
if it's a separate page then it should inherit the base page
Nice. I like how localization is integrated.
lets make this: lapse_interval_seconds and make the default 0
As far as I can tell, we don't have any iframe widgets.
@sonney2k for numerical types, static_cast is equivalent to type(x), and static_cast has the advantage that it is more identifiable in the code (e.g. via grep).

The vector passed for computing hamming distance should be guaranteed to only has value 0, +1 or -1. Or else the results are undefined. If the parameters are already invalid, adding Math::round only gives extra computation overhead but will not make the result right.
please always add new lines before the if
same here - just use Gaussian
The different formatting is to keep the line length below 120 characters, but I have left the previous line in and will remove it.
Not really sure if using a try/catch is the best solution as nothing is being done with the exception at the very least it may be an idea to add in a return to the catch block.
Removing the ```experiment.save``` in playground.rb fixes the double-tracking behavior, which is why I removed it. .. .
Git is a bit confused with this file since I inserted several lines of code and changed the tab level of all the other lines.  I really didn't remove any of the checks, they are all inside the !$results_only if statement.
Generally we alphabetize our css rules per selector so it's easier to scan when you are updating. So in this case, height comes before width. But instead of changing everything now, I'd suggest doing it for future css changes and possibly changing where you have some time.
Are you sure you want to override the parameter ``$predicate``? This often makes code less readable. I also found this on lines 126, 128, and 131.
This code never runs on the server.
Más de paréntesis :P
"layer,one" => "layer, one"
No. Just one dash.
Some funky spacing going on here.
Close. You need to swap the parameter for `every?` and you don't need the `contains?`.
I'm not sure this list is complete (`paginate`,`fine_one`, `distinct`, `amend`, `filter`, `ignore`, `only`, and `order` also take criteria and/or field names) .  If you override [`Pluck::Query#amend`](https://github.com/jnunemaker/plucky/blob/master/lib/plucky/query.rb#L112) you'll get several of these methods for free, but there needs to be a more robust way to get all of the plucky methods...something like the [`Plucky::Methods`](https://github.com/jnunemaker/plucky/blob/master/lib/plucky.rb#L14-22) (which I need to update anyway for #387) but one for methods that take criteria and another for methods that take options ... *or* have abbreviations handled inside of plucky.  Plucky already handles aliasing `:id` to `:_id` (though in two places in the code, once for [criteria](https://github.com/jnunemaker/plucky/blob/master/lib/plucky/criteria_hash.rb#L114-119) and once for [options](https://github.com/jnunemaker/plucky/blob/master/lib/plucky/options_hash.rb#L50-60).)

Thoughts, @jnunemaker @bkeepers ?
i think the concern was more about the tone than the content.
This property should be called facebookAccount
I feel like the variable alignment thing here is more work to maintain than it's worth.
So...

1. luid = self.config.get_id_for_nick(nick)

When the database doesn't exist (and the config object can't execute the request), "None" is returned.

Then

2. if not luid and self.rwdb:

the first part is true (not None > True) but the second part (required since the logical conjunction is "and") fails. So the following line doesn't change a thing.

3. cur_cid = self.rwdb.get_current_channel(None)

The original line is : cur_cid = self.rwdb.get_current_channel(luid). But since luid is "None" at this point, its equivalent.

So... self.rwdb.get_current_channel(None) executes a sql request with None (NoneType object) instead of the user id (int I guess, but can also be a string. I don't care, both are castable to strings...), the None object fails miserably when trying to concatenate the string with it. And that makes the bot fail with a hanful of commands.
Ok tnx... And you're probably right, the installation of mongogeneric was quite problematic, will check this later.
Sorry, I deleted the question because I saw you already answered my question above.

Deleted comment was:
What should I rename default channel_id to?
This was for initial display. You should move this to JavaScript now.
Moved inside `ready` function.
It is interesting to sort by update time. In combination with out pager, this means that for those who open the webpage anew, they will get them in updated time order. And for the rest, they will get updated posts again as they will request them based on the ID of current newest post. This can be useful then to show on the client side that a post has been updated (but we do not necessary have to reorder them).

So, it looks an interesting and useful choice to order like that.
`attr` -> `prop`. I think `attr` is something like deprecated.
(We will minimize JavaScript later. :-) )
Rename `value` to the what you are iterating over (like `panel` in this case).

Do you have to do `$(this)`? Is not `$(this) == value`?
Do you use upper-case or lower-case written colors?
Not the best comment, but OK. :-)

You should probably provide some description for this command so that `manage.py updatehoroscope --help` tells what it is.
Looks like the Whitespace got pretty badly borked, please remove this (you can use `git checkout` to pull the old version)
No need to use self. here.
If there were no implementations of the class, I wonder why it was there... I haven't checked the source control history, but doing so might throw some light on its purpose. Did you check for usage in *all* solutions, or only AutoFixture.sln?

Perhaps the easiest way to figure out what the `AbstractGenericType<T>` class does is to remove it and run all 3000+ tests to see which ones fail.

The unit tests serve two purposes. During TDD, they provide feedback about the design minutiae. Afterwards, they act as regression tests.

My main concern is that the types in TestTypeFoundation aren't really OO types in the normal sense. They are more akin to test *data*. Changing test case data may render some of the regression tests worthless.

If there's a way to figure out that this doesn't happen (e.g. by examining the tests which use the class), it's OK to change the class. Otherwise, I think it would be safer to introduce a new class. This new class doesn't *have* to be placed in TestTypeFoundation - the reason the TestTypeFoundation library exists is because it's shared by the tests of both AutoFixture and SemanticComparison. If the new class is only required in one of those test libraries, it can be defined locally instead...
Adding a request parameter "service" on the request to /cas/clearPass itself shouldn't be necessary - the "ticket" parameter is sufficient.  "service" is required on the request to /cas/proxy to get the proxy ticket, but the CasAuthentication.GetProxyTicketIdFor() call above is abstracting that away.
I haven't tested, but reading this, this looks like, because it uses `server_base_url` will not link bags in their home spaces.

Also, a link to `/bags/<somebag>` will only return a `200` when the current user has `manage` on the bag in question. It's better to link to `/bags/<somebag>/tiddlers` which just requires `read`.

The `server_base_url` information is already in the template as `server_host`, see `tiddlywebplugins/tiddlyspace/template.py`

As a "modificationist" I personally would prefer tiddler.modifier over tiddler.creator.

So, while this is a start, I think there's more to do, perhaps pair up with Colm or Ben to get some consensus on the how and what.
this is no longer valid.
we could...i don't care if you rip it out. i just use it for simplicity's sake -- at this point, i was hoping to just troubleshoot the external call...
No, 180deg in the new one is 'from bottom' (and therefor also 'to top'). That's why the unprefixing here is correct.

I'd like to stick with degree values, the fact that the degree value represents the "from" (clockwise, starting north), but the keywords have to be "to xyz" makes it more confusing than helpful.
Also is element access without check if it exists.
The checking of `ls` in OMZ Robby can be horrendously slow if you open a terminal in a directory with many items. It would be best to point it to a path known to have few items.
This line seems awfully specific - what's up with that?
是【“<literal>--prefix=<value>path</value></literal>&mdash; ”】这句还是下面翻译的内容呢，这个的翻译我调整很多，按照原文直译有些不通顺，你看有没更好的译法~
If with inline CSS things work, this often means that something overrides your CSS. Use Firebug to discover who and where. There is also a switch for Google custom search not to include their CSS, if I am not mistaken? Maybe their CSS is added after yours and this is why it overrides yours?
*case-insensitive*, not *case insensitive* (just be consistent).
We had talked about line numbers as an additional input for purchase details. Decided against it though.
How come that $ sign works here? Another mystery :)
we can't (easily) use bean constructor syntax then
Wow, nice! Will make what looks like really fiddly code so much easier to work with!
what if we need to put 8 elements in the map?  :P
Can make this `private final` instead, but guessing you have a difference of opinion for trade-off of easy of sub-classes doing stuff and code-simplicity to avoid bugs. No strong feelings.
There are more material types. See the previous comment for more detail. You don't need to handle them all, but if a user can load this page with one of the other types it at least shouldn't crash.
_strategy_ pas _strategic_
Effectivement c'est explique dans le tuto, je vais changer ce commentaire la
`id`in this case is not the native Objective-C type. It's the `id class declared in Java which wraps the native type. I think we should use this type to stay as close to the original source code as possible.
Can we not use inline scripts please?
https://jira.mongodb.org/browse/PHP-617
чому в тебе, то з великої, то з малої літери починається коммент докблоку? і до чого взагалі цей комент і інші? він тупо дублює назву методу
why not simply using the Filesystem component to remove the ``src/Acme`` folder ?
Returning `null`, `false`, `true` seems a bit tricksy to me.  Is there a way the code could be expressed so its just a yes/no?
my intent with checkpoint was to offer jobs a way to checkpoint their progress.

Using it at the end to upload the final result made sense.

Using checkpoint to upload multiple jobs makes less sense to me.

However, if an evaluate function *is* going to upload multiple jobs, it also doesn't make sense to me that it should have to *return* in order to report those jobs.  So I know you don't like the idea that bandits can inject trials by side-effect, but I think it is also quite a natural thing to do... this is really an event-based programming style we're doing, with the injection of jobs, and it's ok to fire off events asynchronously with the lifespan of individual processes.

I was actually looking forward to maybe reimplementing experiment as a bandit.evaluate, or at least unifying those two somehow... that would also require allowing it to inject trials / suggestions without returning.
could be a bit more descriptive, i.e. the receiver is nick
Oh interesting - if you dont do the `/*` does it exclude the vendor under assets?
I think you meant by "whatever means necessary."
In Tim, we have a shorter version of this method (which I now suspect means we are all doing something wrong, or there is a bug in ARes, probably the former):

```
def instantiate_collection(collection, prefix_options = {})
          unless collection.kind_of? Array
            [instantiate_record(collection, prefix_options)]
          else
            collection.collect! { |record| instantiate_record(record, prefix_options) }
          end
        end

```
Maybe replace 

    public HeaderAndBody get() throws HttpException, Exception

By:

    public HeaderAndBody get() throws Exception
Gotcha, I left this in only because Carl stated that there was a desire to have everything in an openstack namespace.
rails2 inserts the complete form submit code into onclick.
```javascript
<a onclick="var f = document.createElement('form'); f.style.display = 'none'; this.parentNode.appendChild(f); f.method = 'POST'; f.action = this.href;var m = document.createElement('input'); m.setAttribute('type', 'hidden'); m.setAttribute('name', '_method'); m.setAttribute('value', 'put'); f.appendChild(m);var s = document.createElement('input'); s.setAttribute('type', 'hidden'); s.setAttribute('name', 'authenticity_token'); s.setAttribute('value', 'xxxxxx'); f.appendChild(s);f.submit();return false;" class="close_sprint" href="/redmine/rb/sprints/infrepma/close_completed">Close completed Sprints</a>
```
JSLint will complain about `!-`. If it could be `null` or `undefined`, then let's test for both with `!==`.
И каковы результаты теста?
Хотя я просил тебя написать юнит-тест конкретных кейсов, без обвязки в виде сортировки-мёржа и т.п.
Remove the parens on system calls.
well, are you familiar with the JavaScript `var` statement?
Looking at the code again, I believe that it is coded the way it is (which I admit to copying from is_the_current_page in page.py) is so that, if it *is not* the current page a descriptive message will be displayed about the failure, explaining _why_ it isn't the current page.  I don't think it will always return True, as it won't return True if the assertion fails, it will cause the test to fail with a descriptive message. If I change it to a straight if statement then we lose the reason why we don't think we're on the right page.

Perhaps I am missing something in your comment that suggests doing it a different way that will still retain the message, and if that's the case, please let me know.
次のようには、以下のように に統一しました。

事前にあるコマンドをさすばあいには、このコマンドでは　と記述しています。
Removed this because we're not using it yet.
Cool. So I suppose in order to be compatible with anything up to 10.12.0, and since using symbols (as far as I can tell) isn't wrong, this pull request I suppose is still valid :)
There's no apparent need for `tap` here, since the result of this method is not used anywhere (the goal is to merge/concat stuff). I think it could get a bit clearer by using a simple variable instead of tap + block.
could this be replaced by:

https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/utils/_csgraph.py

``from ..utils.csgraph import cs_connected_components``
use assert_true and pep8 !
BTW, this can be written:

```
y = y.reshape((-1, 1))
```
Rename `err` to error?
`alpha` is not especially explicit to me.
Those attributes are missing: `estimators_ , boost_weights_ , errs_`.
You can use my recently merged `LabelEncoder` for that :)
No need to record.
according to the stat ngin docs, the url should be -> /seasons/:season_id/teams/:team_id/rosters... i imagin this should be scoped to subseason now as well.
Idem here
I could also add a boolean check in `toObjFile` to see if the enum has already been outputted.
I don't think this should compile. It doesn't make sense to have a manifest constant which is initialized to void.
I think the error should be occurring on these lines, not on the assignment.
(So I think the bug report is a diagnostic error + accepts-invalid, rather than rejects-valid).
Should we get rid of the _xxx suffix in this comment?
I naively grepped Text.Blaze for ":: String". Lesson learned. :)
This is a bit of a redundant test.  If you were going to test you would need to check that 

expr->getOperator() ==  no_constant

but that would catch identical cases to the assertex(value) below.  If you are going to guard the function then it would be best to test for no_constant and remove the assertex on (value).
This is a problem, these tests require that a Jenkins instance be created, which is why they inherit from HudsonTestCase. This is probably why the tests have a NPE when running.
The convention in socorro-crashstats is pep8-like variable, function and method names that use lowercase_and_underscores. Classes are capitalized and then CamelCased.
aha... I see...
I'm not sure I understood comment about protocol flags here as I don't actually know what the chrome flag is for, but in any case I'm happy to remove it.
Definitely do not want to ship flowvisor with debugging set to TRACE as default.
Not really.  The issues is created and viewable.  Most services will still work right now.
missing 3 `@param`
remove all hard coded values.
http://docs.puppetlabs.com/guides/style_guide.html#resource-names
"All resource titles should be quoted. (Puppet supports unquoted resource titles if they do not contain spaces or hyphens, but you should avoid them in the interest of consistent look-and-feel.)"
Can MSF_LICENSE be used?... It applies for all these SOAP related modules. I'm not going to annoy writing it more times.
I've tested only linux platform, but looks good to make it compatible with these platforms if anyone rejects.
its not about if the file is readable, its about if the file is "includable", ie. if all classes referenced in the file actually exist or not to prevent a fatal error. but the approach here doesnt solve this issue unfortunately.
Another thought: maybe it's not even worth checking arguments and let the Image class raise whatever error it would normally?
@garyc40 probably You are right at some point, but if this function would be only for WP, the notice would state "...deprecated since WordPress version..." and not just "since version"...

Anyway, this notices are not for 'end users', so if I see "```woocommerce_product_taxonomy_content()``` is deprecated since version 1.6! Use ```woocommerce_content()``` instead.", You can believe me or not, but I'll not do search in akismet folder ;)
that makes perfect sense... can't believe i totally overlooked that!