labofsahil/github-event-dataset-2012 huggingface dataset:
@drogus If we don't check for @stale_state.present?, an another testcase is failure.

```
 1) Failure:
test_associate_with_create(HasManyThroughAssociationsTest) [/home/kennyj/rails/activerecord/test/cases/associations/has_many_through_associations_test.rb:394]:
3 instead of 2 queries were executed.
Queries:
INSERT INTO `people` (`best_friend_id`, `best_friend_of_id`, `comments`, `created_at`, `first_name`, `gender`, `lock_version`, `number1_fan_id`, `primary_contact_id`, `updated_at`) VALUES (NULL, NULL, NULL, '2012-03-30 17:52:17', 'Jeb', NULL, 0, NULL, NULL, '2012-03-30 17:52:17')
SELECT  `people`.* FROM `people`  WHERE `people`.`id` = 47 LIMIT 1
INSERT INTO `readers` (`person_id`, `post_id`, `skimmer`) VALUES (47, 2, 0).
Expected: 2
  Actual: 3
```
Thanks. Fixed with `ActiveSupport::Deprecation.silence do`
Just a question: before it was being tested for `primary_key(from).to_s`, now `to_s` is gone.. is that a problem or everything is working fine - ie all tests passing?
Excuse me, but is there a choice? This method is already removed from the next version. The only other choice i see is to remove it here and to deprecate in the previous version...
Double space here also :codestyle_nazi:
this particular line makes the assumption that .and is always the 'join' for clauses.  This is true until now, but if one had a scope with or joining to another scope with ors, they'd end up ands.

If there's an arel 'join clauses' method which would use correct concat here that would be handy.  If not, a hint at where to find the concat used in the where to extract a method here to do it would be good.
Oh this one is good.
thanks for the ping, @VladUreche, I'll make sure it doesn't cause trouble when cutting M6-virtualized, but i'm sure it doesn't need more special treatment than the other special files
`##` and `==` are final, so they are handled by other checks.  I think equals/hashCode should be disallowed for universal traits as well, though.  I think that is out of scope for this pull request, though.
This normalization only seems to be done when ``analog=True``, right?
unless check[:handle] == false
  event[:action] = is_flapping ? :flapping : :create
  handle_event(event)
when is this not sufficient? (I couldn't access link referenced)
Ah, I see.  Well, in this case I'd recommend either turning the two lines into assertions nevertheless, that is:

```Python
assert str(rebuilt)
assert rebuilt.is_commutative
```
or adding your previous comment to the code.  The reason I ask for this is that, otherwise, it may be somewhat difficult to figure out the exact purpose of this test function.
remove view hide here aswell?
I think may that `scale_dimension` should just be `expand_range(scale_something(scale), expand[1], expand[2])` and `scale_something` is the method - not sure what it should be called though.
Is the "clone" needed... a user can decide if he needs to clone his CDbCommand in the call to CSqlDataProvider
But is it active at the init() time?

How about changing both to be the same "A valid ..."
Yes, you are right. We shouldn't change the default behaviour. Even if it's broken at the moment.
The identation here is wrong. Also, if possible can you put the content of the for() block in a new line?
I might even go a step further and make the exception type optional, so that `with assert_raises():` and `with assert_raises(Exception)` are equivalent.
Do we need to assign f, m and t here or can you _ them out?
Aha, I was going to add a comment when yours just appeared by magic! Awesome!
@fetep suggested wrapping the auto-create bit in a config option. That could be on by default for dev but disabled in prod if that makes him happier.
Why do you use @LOG? Redundant use Log instead.
this => This
strcmp? what?
entirely redundant to the read document. if db.getCollection() isn't fully documented, or is lacking somewhere, some of these details can go in that document.
The short explanation is that this code looks for the first paragraph *without* a parent, i.e. not wrapped in <div class="warning">.

The parent check of 2 is added because pyQuery adds <html><div> to wrap all content.  On the URL cited above, the SEO description should be:

          The animation-duration CSS property specifies the length of time that an animation should take to complete one cycle.
NIT: same indentation nit.
Great, nice test!
@imkingdavid you never did this
Should probably not all be on one line (i.e. add the event to the next line)
style_version is only for style author's reference, it isn't used anywhere in phpBB, so I don't see a point in making it mandatory.
Do we really perform user permission checks in mcp?
That simply shouldn't be passed to the function. I could also write some code to pass the database credentials to it, but I'm not going to write anything to stop people from doing it!
Not sure if we want inline event handlers like this.
Should probably throw a 404
Wouldn't it make more sense to do this logic in SQL?
Sounds something new after the rebase...
why does the id reference project.name instead of the formatted one?
typo here I think.  utils.request_maker returns undefined (which is a little strange, but probably won't break anything).
Remember that logLevel does not work if debug is not set to true.
This isn't purely login... although login will call this, this can be used by any class that wants to check if the authentication is correct
want to retry in case a signal interrupts the system call.   will add comment
Wouldn't it be simpler to do the same for any character after a backslash if it's not at the end of line? Why make a special case for \\ and \"?
"DO not inherit from AdhocPlannedStmtBatch" but do not add redundant code, either.
Instead, add a boolean m_explain member to AdhocPlannedStmtBatch and AdHocPlannerWork and eliminate the two redundant ExplainPlan... classes. Set the member to true for "explain", transfer it to/from the Work object, and use it in processFinishedCompilerWork to decide what needs to happen next.
Don't include status and message in the return value. Just return the next hostname.
I won't respond to the remaining instances - but definitely not a fan of the word "useless" - better to describe how to make it better!
Feel free to suggest any improvement/simplification. I see now that Vinay used it once in his branch in _doctest.py. I have chosen another route for updating _doctest.py and propose to copy python 3.2 doctest.py in _doctest3.py, reapply Python  #1521051 on it and import conditionally _doctest.py/_doctest3.py. It's difficult for me to evaluate if this function might be useful or not in Django apps ecosystem.
Should be ``.. classmethod::``
`sys.getfilesystemencoding()` may return `None` under Linux — see [Python's docs](http://docs.python.org/2/library/sys#sys.getfilesystemencoding). You need a fallback.

`django.template.loaders.app_directories` uses `fs_encoding = sys.getfilesystemencoding() or sys.getdefaultencoding()`. You may also hardcode `or 'ascii'` and `or 'utf-8'`.

I don't have any strong arguments one way or another. If I had to choose, I would use `or sys.getdefaultencoding()` for consistency with existing code.
Good point - we already do this for Sonar builds, so indeed it can be easily integrated.
May wanna use something like this instead:

```js
grunt.file.expandFiles( "test/unit/*.js" ).map(function ( path ) { return path.match( /^test\/unit\/(.+)\.js$/)[1]; });
```
All of this code was changed to support this?
This is terrible to read. 

Ditch the underscores.

Needs padding spaces for readability
did you run this through jshint? `glue` is leaking to global. also, can't you do:

`glue = rmultiplebg.exec( posX ) || ""`
Unnecessary, as the `node: true` option covers this already.
we use a text to unique identify an itemset (svec) rather than its hash key.
For example,  we have three items: 'a', 'b' and 'c', we assign 1, 2 and 3 to them respectively. for an itemset '{3}:{1}', the text (hash_text) is '1,2,3'. That's we first get the positions of the elements whose value is 1, and then concat them to a string with comma. This string is the hash text, and it's unique.
we are removing the functions in the libmana after they got emitted once. So maybe clean them here too?
Though of course not needed calls to the libmana do not really hurt... Also it might be hard to keep multiple register requests...
So probably just keep it like it is. Would be a different patch anyway.
I didn't... That's the other difference between Fjord and Kitsune. With Kitsune, it was crucial that search never goes down, so we needed to be able to reindex non-destructively. With Fjord, we don't have that requirement, so I removed those bits, too.
> Right :-) And since there's only read-write and read-only to choose from (?) we should in principle only allow updates if the user also has permission to delete. In that case I think it's better to throw an exception because the alternative would be too restrictive. One can always blame the CardDAV clients if they barf on it ;)

This is only true because of the default 'simplification' of getACL. If you override 'getSupportedPrivileges' you can create custom structures. Delete is represented as `{DAV:}unbind`, but `{DAV:}unbind` is represented on the collection. WebDAV ACL considers deleting items an operation on the parent collection. The default structure can be found here:

https://github.com/evert/SabreDAV/blob/master/lib/Sabre/DAVACL/Plugin.php#L354


In theory should not have to override the ACL plugin.
Best to remove this line to avoid confusion about the version
This index is to long for certain mysql setups which have a maximum of 1000 bytes (333 utf-8 characters)
See comment about the import ordering that I started using.   basically I moved r2.lib above r2.controller because r2.controller was package local and r2.lib was an non-local package
Also, `Subcommand posttype` should become `@subcommand post-type`.
然后我觉得你应该忽悠更多的人参加创作~~~


On 2012-3-31, at 上午1:12, Yihui Xie wrote:

>> @@ -200,6 +200,13 @@ $ which R
>> $ echo $PATH
>> /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
>> ```
>> +当然,奇葩的make -j8可以不用安装(sudo make install)就能使用R,你只要在`~/.bashrc`后面加上PATH就可以了. 更改`YOU_R_PATH_Directory`为你的R目录,复制如下内容到你的Terminal中:
>> +
>> +```bash
>> +cat >>~/.bashrc<<EOF
>> +export PATH='YOU_R_PATH_Directory':$PATH
>> +EOF
>> +```
> 
> 这个理由可以接受。既然到了那种奇葩的地步，那已经不是我这里计划涵盖的内容了……
> 
> --- 
> Reply to this email directly or view it on GitHub:
> https://github.com/yihui/r-ninja/pull/2/files#r629512
Its own line 6?
That would be great to have.  Don't kill yourself, though.  Let me know when you think we're in good shape test-wise.
What is a sr? I don't know anything about this code; maybe it would be obvious if I did? Or maybe this could be a little more descriptive.
Now that we have this, do you want to make use of it by setting the "Experimental" item to disabled?
This call in turn calls loadAgents() and _loadedAgentNames increases by ten each time we call it from here. To avoid that we also need to call _onDisconnect() before this call. But I think it is better if we can add a new method _reConnect() as follows.

###
function _reConnect() {
        unloadAgents();
        var promises = loadAgents();
        _setStatus(STATUS_LOADING_AGENTS);
        $.when.apply(undefined, promises).then(_onLoad, _onError);
}
###
Yes, indeed. I think even I may forget this case later if there is no comment.
Bootstrap uses `<a>` for dialog buttons instead of `<button>`, so currently this isn't a problem.  Do you still think we should change it?  I suppose we could filter only on tagName (allow `<input>` and `<textarea>` and maybe `<select>`, but nothing else?).
As a small stylistic issue, we've generally been using "0" instead of "0px".
I was struggling with these names too, but felt adding "callback," "func," or "function" to the end made the arguments too long. I like the "Fn" suggestion!

Ty 

-----Original Message-----
From: Narciso Jaramillo [mailto:reply@reply.github.com] 
Sent: Wednesday, May 23, 2012 10:53 AM
To: Ty Voliter
Subject: Re: [brackets] Menu, MenuItem, key bindings, and Command API (review only) (#927)

>      var _commands = {};
>  
>      /**
> +     * @constructor
> +     * @private
> +     *
> +     * @param {string} id - unique identifier for command. Plugins should use the 
> +     *      following format"author-myplugin-mycommandname".
> +     * @param {function} command - the function that is called when the command is executed.
> +     * @param {?function} isEnabled - callback function that returns true when 
> +     *      the command is enabled.

I think we should rename "command" and "isEnabled" to make it clearer that they're callback functions (and in the case of "command", to avoid confusion with how it relates to the Command object). Maybe something like "executeFn" and "isEnabledFn".

---
Reply to this email directly or view it on GitHub:
https://github.com/adobe/brackets/pull/927/files#r869650
Yes, it sounds like this should be logged, if it is not already.
they will be customizable per domain
updates to planning-related tests?
There may be too many "is" in this sentence ;-)
None of these parameters are described. It may be very hard to understand what they are used for...
35, but yeah. I was thinking about making the last “digit” a single value (since 0 is the only value for it that we're interested in) and allowing an arbitrary number before that so that we can queue up as many requests as needed (though we'd trigger some interesting performance issues if you ended up with too many requests stored on the server…).
it still didn't reverted. Perhaps you forgot push to dev again.
Wouldn't something like this be simpler:

```
if 'code' in request.GET:
    user = auth.authenticate(token=request.GET['code'], request=request)
    auth.login(request, user)
    # message user that she has been logged in (maybe this will be already in auth.login once we move to MongoDB
else:
    # message user that she has not been logged in because she canceled the Facebook
return super(FacebookCallbackView, self).get(request, *args, **kwargs)
```
Spacing after `()`.
Ah, should have read this first.
A bit from column a and a bit from column b...
omgwat!
I refactored this method and i forgot to removed that line. I've taken it out
This applies in general to commenting out code.  It's fine during development but we should clean it up once we know we don't need it.
@whiteinge So I changed this to True, because virtualenv is changing their default behavior to using no-site-packages

I made the change on the module(https://github.com/fxdgear/salt/commit/f4c587218924cbb62cd2e643f394791c789c7f21)
but didn't recognize that it should have been done on the state as well.
You're not an idiot
Gonna fix this
is this really how our formatting rules format files?
It's to create packages which are 'tagged' with the current running kernel, so that you can have the same version built, but for different kernels and they don't overwrite eachother.

I told brian to ignore this patch, but take what he want/need.
third occurrence of the same lookup in this function - perhaps ignore my comment at line 878 and instead stick the result if this lookup+get in a vector?
Better to use %s instead of named parameters since known to work for extraction for translation.
you don't set ``$this->space``
I'm still not sure how to do this properly to avoid race...my async skills are a bit lacking...
Done 6f7d9e1
Wow.  Maybe that's why I couldn't get a homescreen up!
虽然我现在看到catch以后什么都不做会敏感一下，但是我得承认。。。我还真不知道这里该干点什么>_|
since i'm asking for a change above anyway, let me be super nitty and point out that parenthesis are not needed in the new return value :)
Yay!  ACK
See my update below for a better version. Also a small grammer fix.
See below, we now return error() on a failed check.
Isn't this change contrary to the other changes? You are adding an assignment instead of removing. The line remains too long regardless.
To have the WMS services (not groups) closed has no seance for me, without that, after the GetCapabilities are loaded, we have just one closed node.
Should be removed. The en translation is in the original prototype already. And this file will be removed (see @sbrunner's work on i18n).
this would mean that all your libraries have to live in the same folder, which is absolutely not a requirement in PSR-0, and will be wrong as soon as you use some vendor libraries in your code (as you will not mix the repositories together)
Can these "toggle-yet-to-vote" links be turned into a single link?
`path` is leaking globally
can do an organize imports run to correct this static
Docstrings for all functions!
You can write straight to `note_desc.blob_oid` and save some stack space. `target_oid` isn't needed
Fine, "Path\to\cmake.exe" is fine with me. `-DTHREADSAFE:BOOL=ON' is also fine. But then, why not to make it default. 

Not everyone has cmake.exe in path, like me. If you download lates cmake and install it, by default it does not add cmake.exe into the path. You have the option to do so during installation, but by default, this option is off.
static missing.
one more nice enhancement would be to allow dynamic vars even in the inline partial declaration just like how we allow in  {+../}

Another issue though.
I think it would be more appropriate to use the q element here http://dev.w3.org/html5/spec/the-q-element.html#the-q-element
No, I'm dumping these values as the first action within saveScreenshot. Unfortunately JSON.stringify didn't work for me as it gave the following error:

> [JavaScript Error: "TypeError: cyclic object value" {file: "resource://mozmill/modules/frame.js" line: 650}]

So I used the following snippet:

    seen = [];
    dump('frame.persisted:\n');
    dump(JSON.stringify(frame.persisted, function(key, val) {
      if (typeof val == "object") {
        if (seen.indexOf(val) >= 0)
          return undefined
        seen.push(val)
      }
      return val
    })  + '\n');

Taken from http://stackoverflow.com/a/9382383/154975
if id is given, this should be detail not list
If leaving commented out code in, I'd love to see an explanation. E.g. 

```python
# This is how you'd do to run in debug mode
#foo.bar()
bar()
```
Since `nb_home_mobile` is only used in mkt/home, do you think it would be better to override `setup_featured()` in `TestHome` rather than having it in here?
r?
This should be indented
Agreed, when we provide `TryParseAndVerify` on `ResourceLinkVerifier` for completeness sake we should also provide a `ParseAndVerify` method. As this no longer will be a convenience extension method it will throw a normal exception.
Yes this isn't fun.  Forcing a hash params payload would negat all this.
Don't change the style of the original code, the patch could be rejected because of it.
Can this script run right past the available data?  If so, if you didn't want a python loop for it, I think the bash loop would be something like:

    $year_end=`date +%Y`
    for year in `seq 2000 $year_end`                                                
    do                                                                              
      for day in 60 120 180 240 300 360                                             
      do                                                                            
         printf "%d:%03d\n"  $year $day                                             
      done                                                                          
    done
I think this should be part of the condition above because if the session has not started, surely there is nothing more to do here at all.
this will conflict if someone creates a type named ``input``. you should not use a block named ``*_widget`` as this matches the convention for blocks of the theme
should be `Checks`. Why `extra`? and `defaults`?
calling class_exists('Symfony\Component\Security\Http\Firewall\DigestAuthenticationListener', true) should trigger the autoloader :)
properties should be initialized in the __construct method
coding standards: `LAST_USED`
I'm not sure how to tell that it supports mixed values, either string or objects
Wouldn't it be better to create a compiler pass class for that logic directly in the DependencyInjection component?
How about the following bootstrap then?

```php
<?php

/*
 * This file is part of the Symfony package.
 *
 * (c) Fabien Potencier <fabien@symfony.com>
 *
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 */

spl_autoload_register(function ($class) {
    if (0 === strpos(ltrim($class, '/'), 'Symfony\Component\BrowserKit')) {
        if (file_exists($file = __DIR__.'/../'.substr(str_replace('\\', '/', $class), strlen('Symfony\Component\BrowserKit')).'.php')) {
            require_once $file;
        }   
    }   
});

if (file_exists($loader = __DIR__.'/../vendor/autoload.php')) {
    require_once $loader;
}
```

By running `phpunit`, you'll get:

```
...........SS......S............................................. 65 / 82 ( 79%)
.................

Time: 0 seconds, Memory: 4.25Mb

OK, but incomplete or skipped tests!
Tests: 82, Assertions: 156, Skipped: 3.
```

By running `composer install --dev` first, and then `phpunit`, you'll get:

```
................................................................. 65 / 82 ( 79%)
.................

Time: 0 seconds, Memory: 4.75Mb

OK (82 tests, 160 assertions)
```

What do you think?
Btw, you can remove the method entirely as it can be inherited from AbstractType
Consider named parameters.
inke `:` nadare va hame too ye radige kheyli khoobe
如果你觉得没必要也行，我是记得好像模板里也有个类似的问题，如果有必要分开，就再改吧
As for me, it is not enough clear  in this snippet whether the usage of Alias<T> is recommended or not!
Nah, like I mentioned, I think I'll just let it slide.  If people complain later (doubtful) then we'll revisit it.
Why the change to use of class variables here over the instance variables from the base?
see have_same_elements?
I think I'd prefer this (and elsewhere) to be two separate `step` lines. Especially since I found documentation suggesting how I'm supposed to do the table in player_setup.rb
well, subclassing is fine, but as far as I can see, the call chain is `sendToGanglia`, `private void sendMetricData`, `protected void sendMetricData`. the private one sets the first argument to `getHostLabel()`, so the condition is always true, unless I'm missing something. or are you saying that I should override `protected void sendMetricData` in my subclass as well?
I could not properly test this because of a bug (?) in tree menu, but it seems to me that this doesn't allow inivisible tags to be hidden, which should be possible.

About the bug, When I hid one tag in treemenu, suddenly the parent of that tag in the treemenu stayed forever with clock icon next to it, meaning the request never finished.
But now links like (http://blah.com) won't work
i really think it should say New Session so that the user understands what they are doing, they are not saving the previosu session, the detaisl are copied from the previous session incase they have a lot of similarity between their sessions.
Did you mean to reduce the number of checks?
Strangely, without these specific margins, the textbox is slightly under the label.. and this is disgracious.
Yeah, it doesn't have to be a required field indeed. Will fix this eventually, but if someone wants to do it for me, please :-)
why so many comments :)
hmm, nevermind
same remark here, would that work?

```javascript
// XXX remove network.capabilities when bug 790231 lands
var keys = network.security ? network.security[0] : network.capabilities[0];
```
Great catch! I will fix it
I wonder if you can use 'defer' on some of the scripts
Note: that’s the main reason why I don’t merge your patch as is.
Can you put this into `keyboard.css` ?
nit: indent seems weird
nit: 0rem -> 0
I tested but the onerror callback is not fired when input wrong pin, could you check this?
We need a warning here since we're changing the user input and sending a SMS to a number != than the one he typed.
(I understand the network is doing that kind of stuffs too)
Yeah.  The Thunderbird implementation of comparable (although much less sophisticated) logic just split the lines on the newline boundaries and it was a little less horrible because there were fewer indices to maintain.

The good news is that if you look in gaia-email-libs-and-more, you will see there a fair number of unit tests for this.
typo: `isShown`
wouldn't it be better to do an `Array#indexOf` check on the `deptree` to see if has been added before?
Can you add a spec (similar to this) as well?  The cukes function mostly as documentation, and we rely on the specs to protect against regressions during refactoring.

Also, given the use of the cukes as docs on relish, it'd be nice for the example here to show a more idiomatic usage of `subject!`.  I think this functions well as a test but doesn't really show a case where I'd actually recommend `subject!` be used...it seems odd (and a bit convoluted) to create the subject object in a separate `let` block and just use `subject!` for a side effect on that object.

So...if you can think of a more idiomatic example to use here, that'd be great :).
Trying to improve here:-

https://github.com/skyluc/scalatest/commit/ba9f6d939a24c0f1bb6f8f82c3e542ae00dee08c
This patch is bad. `av` is not set here.
Is commandManager.canUndo() basically idiomatic for detecting a new goggles session? It might be nice to give it a more clearly named helper.
Can we keep this option as 0 (as it was before) -- otherwise every existing installation will break.
You have some tabs here. Also you can write the whole thing like this:

```php
return in_array(false, $array);
```
Ok, let me think about it.
I had a look at the config here.  The default of `None` is because the service is disabled by default, mostly because we don't have a good default port to run on.  So to activate the www you need to have some stuff in your buildbot config.  I think you've got this set up now?  Something like
<pre>
c['www'] = dict(
  url='http://your.ip.here:8080',
  port=8080
)
</pre>
so this change should be reverted from the patch.
This intersects the discussion with Dustin on first commit. 
I'd like workdir to be initialized from repourl (with escapes or hashing), but it shouldn't change if the URL does,  because that'd pull and re-emit all changesets (the latter because object id is based on it). I'm a bit puzzled about what'd best. What do you think ?

On the other hand, improbable events such as a circular permutation of repositories (very undecided maintainer) could lead to a serious mess.

btw, the docstring in getStateObjectId is outdated (no mention of branch)
I'll land a change for this shortly.
I think there is a spacing issue here
Why do we need to link Xrandr?
Nope, oops. I guess that was from the merge. Thanks!
Translatable please! :)
okay, I'll do it
This was an interesting bug that would work differently in Eclipse vs Maven (based on what other tests had done to the state of the referencing subsystem). 

By configuring AXIS_ORDER the equalsIgnoresMetadata check is now fixed
Use default parameter if not set by params hash. If you know less ugly way to do this, please tell me how :)
@randx Got it.  I did not yet switch to gitolite 3.
Out of curiosity, why do you check for negative values?
Yes, this is missing. Adding this.
yes, i think you should do a MOD_RES_DENY for good measure here.
you don't need to specify the default in the comment; the doc generator figures that out ;)
What if you want to show the current page number?
copy paste error.  fixed.
Is this code removal necessary for switching to the bottom row? Keep this as a separate pull request if it can switched to WindowTracker by itself.
PEP8: Add a new line to the end of the file.
The footer link have been replaced with original one.
as per exporting?, what value does this have in the DSL?
Instead of this, you should inherit from Puppet::Error, which already includes this functionality.
See <https://github.com/puppetlabs/puppet/blob/3.x/lib/puppet/error.rb>

The log_exception method also will automatically print any nested exceptions.
Is there still a reason for the existence of this method?
What about mock realm that throws new RuntimeException() directly - this would protect against the implementation merely handling the two shiro exceptions explicitly. I understood the point to be that any RuntimeException gets ignored, not just Shiros
See [this comment](https://github.com/zendframework/zf2/pull/2029/files#r1256653). ErrorHandler extends Simple and I used `static` to refer the overridden default in this constructor. I also removed `PHP_EOL` (see [this comment](https://github.com/zendframework/zf2/pull/2029/files#r1256662)).
@weierophinney Since my latest PRs about Zend\Mail $this->headers has always HeaderInterface instances. So this is correct.
Can you remove the trailing whitespaces on this line please?
The original has two routes for the same view (fine), one outside the basePath. It's not a pattern I'd generally encourage but wanted to keep the current behavior.
Whitespace-only change, please change back!
so the canTick logic goes into onTick?
Could you name k to what it really is? I have no idea what's going on here. Lets leave minification to minifiers :)
I still need to do something to make this amd compatible.
I thought a space was proper?
Would be better to compare names rather than index. What if I move around the links.
There's only one type, so it's moot specifying it. Can you remove it?
`{collection: 'todos-agilityjs'});`

->

```
{
    collection: 'todos-agilityjs'
});
```
i understand your point here...

because we are talking about a relatively easy function, i just moved it directly to the template as you suggested, accepting that little non dryness...
Very true.

db:drop clears al the collections, but db:create and db:schema:load are no-ops. It'd be good to keep the db:drop to ensure we're working in a clean database in CI but the others can go.
get rid of moduleclass (base is default anyway)
this should be in _recv, not in send?
in easyconfig files, we rarely follow the pep8 style

not sure why

but I'll correct it
This line should not have a trailing comma. It's causing the [build]](https://travis-ci.org/#!/marijnh/CodeMirror/builds/2726045) to fail. There may be more issues--don't have time to look now though. It would be awesome if you could get these small issues out of the way before Marijn takes a look at it. (Each time you push a commit to the branch/pull request, another build will be tested, which takes about 30 seconds.)
Spacing between CSS ancestors and their descendants, please, s/>a/> a
yah, that was easy enough to do with questions, but with KB articles, there were so many classes involved i decided it'd be obtuse.
at least one of the test cases also does a Assert.contains for the article name in the title, so i'm less worried about it.
Usually the convention is to use `async.map` if the mapper function is performing an async operation which is true in this case.

I will modify this place to use `async.forEachLimit`.
Sorry, I messed up my previous answer: the two fill_* functions *are* blocking!
third time you create the same url. Make a variable.
I also moved this line above .respond. It will be called second time after .respond, which I think is actually sinon's issue - the request has been aborted by the time we call .respond, so it should not probably do anything. With real ajax requests the failback will only be called once - immediately after trigger('route').
We should also pull tip here to get all branches. Necessary for the future when we want to support Mozmill 2 scripts.
Alternatively, could we move this into `launch`, and have an optional argument for `url`? Would we want to launch an app without switching to it's frame? If so, perhaps another optional argument such as `switch_to_frame` that defaults to True.
We could do this, but neither of those functions exist in the connector. This would need to happen in _itemDone.

There is, however, a slightly more elegant solution than the above. The href property (but not attribute) on an a tag is always a fully qualified URI, so:

```javascript
if(!havePdf && (pdfLink = ZU.xpath(doc,
		'//div[@id="viewing-options-links"]//a[text()="PDF"]')).length) {
	item.attachments.push({
		title: 'Full Text PDF',
		url: pdfLink[0].href,
		mimeType: 'application/pdf'
	});
}
```
https://confluence.skunk-works.no/confluence/display/eng/Java+Coding+Conventions#JavaCodingConventions-Accessibilitymodifiers
But there is no whitespace changes like in other whitespace deletions...
This line needs a comma at the end.
`echo #{package}`?
"pair of curly braces" ... with an zero-based integer? Might want to describe this, and even give an example. What happens if the # of parameters doesn't match the number of variables in the message? (Our logger fails, but I don't recall what happens on regular string formatting/substitution.)
song.lpd can return None
needs removing now that the config option is gone
Okay, I admit that _almost_ makes sense, given that it works "the other way around." (ie, 0 is falsy)
Replacing `{Tap}` with `{T}` should happen in `to_symbol`.

I guess there's no shorthand for `{Untap}`?
`if inputs is not None` ?
I removed the "onyx-affirmative" and "onyx-negative" classes from these buttons during the UI cleanup, to make them less obtrusive. I think saving colored buttons for exceptional situations (like deleting your files) works better.
if ( !empty( $embeddedObjectIdArray ) )

;)
To avoid the raise of a PHP notice maybe we could declare $this->writeFlag attribute as : protected $writeFlag; ?
Way to much. At most 5, less would be better. Plus, `.substring` should be used instead of `.indexOf`, as I care about speed.
а на это где тесты?
's' is a local variable
I think it would be better to keep the parser error handling in one place. A better solution is to assign the value of the parse_task function to a single variable, validate it, and then split it.
Again, I don't think this shortcut is worth the confusion.
I believe this is now unused.
So please remove all of them so that we are doing it right.
@nicolaasmatthijs Agree that taking the password out of the Principals CF is a good thing to do, I'll move forward with this schema.
When multiple tenants get started up, won't this result in duplicated handlers on the global server? e.g., if you have 5 tenants up, you'll end up with 5 instances of these handlers all responding to /api/modules on the same request.

I think Nicolaas alluded previously that this could now be put in init.js, so that you know it only gets executed once.
Using `i+=2` seems prone to error. if there is any bug that causes data or visibility to not be there, then data aggregation will be out of sync, and you could have incorrect 'visibility' values being read (i.e., visibility of xxx becomes the data of yyy), which could impact profile data security.

Would be more stable to determine the values based on:  `name.split("#")[1] === 'visibility'`
This should all be converted to Validator.
Thinking about this, it might be a bit strange to move all these things to oae-rest, given that we're trying to be modular and 3rd party oae modules can exist. This will be an explicit item for next week, so we can leave it until then.
drupal_get_path uses the machine name and as such should remain lowercase
Why is 31557600 changed to 31536000?
теперь вижу. когда проверял - поставил после каждой переменной nslog и вывод показывал что они всегда одинаковые, но сейчас перепроверил - так как вы говорите. или магия или я натупил)
m - это проценты, а не количество, вдруг в ДНК у вас будет не 100 элементов?
Никак не могу понять, а prc++ - это зачем?
интересный способ. такой еще не видел.
Думаю у вас в этой строке ошибка 
Должно быть 

for (int i=0; i<X; i++) {
        [DNA replaceObjectAtIndex:[[[positions allObjects] objectAtIndex:i] integerValue] withObject:[Cell randomElementWithout:[DNA objectAtIndex:[[[positions allObjects] objectAtIndex:i] integerValue]]]];

    }
ИМХО в нашем случае лучше было вообще сделать длину ДНК константой, это избавило бы от необходимости проверки длин ДНК разных клеток в hammingDistance.
В нашем случае n это не количество элементов, которые должны мутировать, а процент. Тоесть, например, если инициализировать Cell 9-ю элементами, а потом передать в mutate число 73, то программа при запуске виснет.
This will break the bindist target, because it exports the source from git using git-archive, and then builds it from that clean tree.

Could you separate this out from the other changes?
еще хорошо бы проверить что нибудь на страничке, типа флеш мессаджа или специфического контента (например содержит новый емейл   и не содержит старый)
As a quote from me when you said this before:

>Hmm... Methinks you be wrong. GenericTool, etc are all just stubs.
Heh, ok. Doing that.
@mlavin Someone has a PEP8 checker in their editor!
Please define the FQCN here
不错。。取出来的this-handlers不会变了。。
In order to make the package as eminently goinstallable as possible, all of the generated files are actually checked into git.  You should be able to run the test.sh script to generate them the way it wants to.
nice attention to detail there, thanks for the tip on the ,    I was curious about the apparent ambiguity
you can get rid of the commented lines if the new one is correct
this will modify the instance right? unless you do a copy of info first.
please when returning an error to the user create a JSON object in this form;
```
{ statusCode: <status_code_of_error>, message: "<error_description>" }
```
Do this here and anywhere else that returns an error.
Why is there to 'http' require statements?
Let's avoid promoting anything with a prefix :D
You only need one of these tests
Yes good idea. I will make the change.
Good idea, a bit irritating but fixes the warning. I am hoping there is
better solution.

Sent from my iPhone

On 6 sept. 2012, at 11:21, Julien Richard-Foy <notifications@github.com>
wrote:

In framework/src/play/src/main/scala/play/api/libs/json/Util.scala:

>        )
>
>      def join[A >: A1](implicit witness1: <:<[A, A1], witness2: <:<[A, A2], fu: ContravariantFunctor[M]): M[A] =
>        apply[A]( (a: A) => (a: A1, a: A2) )(fu)
>
> -    def tupled(implicit v:Variant[M]): M[(A1, A2)] = v match {
> -      case fu: Functor[M] => apply{ (a1: A1, a2: A2) => (a1, a2) }(fu)
> -      case fu: ContravariantFunctor[M] => apply[(A1, A2)]{ (a: (A1, A2)) => (a._1, a._2) }(fu)
> -      case fu: InvariantFunctor[M] => apply[(A1, A2)]({ (a1: A1, a2: A2) => (a1, a2) }, { (a: (A1, A2)) => (a._1, a._2) })(fu)
> -    }
> +    def tupled(implicit v:Variant[M]): M[(A1, A2)] =
> +      // SO UGLY UGLY UGLY workaround for unchecked type erasure warning... no cleaner way found...

Isn’t it possible to pull the tupled method into Variant[M[_]]?

—
Reply to this email directly or view it on
GitHub<https://github.com/playframework/Play20/pull/440/files#r1542359>.
this part should be a separate chapter as it is not only useful for compiler passes but for all places dealing with the ContainerBuilder
Typo: `ppear` needs to be `pear`
If your sentence before the code don't end like this:

    foobar:

You shouldn't use the shorthand. Please replace this with the `.. code-block:: php` variant.
I did not intend to lose a line. Any idea where it went or which commit lost it?
Are we expecting anything but one Camel?
How do users use this from Java?
@cebe Agree with what you said. Thanks.
LIkewise, as above.
I have to admit, this bit is rather cool. Especially the fact that the routes are now located *with* the controller really does help speed up development. Good job.
I _really_ think we should use isoformat time, in this particular case, the code would be

```
datetime.now().isoformat()
```
default success is false
This test can be removed as `Logger` is passed on constructor and never changed
It should be `text-summary`, not `text-only`
No, that looks like a leftover
Should extend the `JSONResponseMixin` too and...
Does format 3 not need to support "frame" format? Is it also generate by textpacker?

How to generate format 3 plist files using textpacker? I only know how to get format 0, 1 and 2 from it.
format fail?
I don't really know _why_ it takes a pointer as an argument, but it'll have to copy all the values from it anyways, because there's no guarantee that the caller's struct will exist as long as the diff API needs those values.

In other words, if we can free it right after calling `git_diff_index_to_tree` (for example) without issue, it could also just as easily be on the stack.

CC @arrbee
I would annotate this `strong` or `copy`, just as a good practice with object properties (even if it's never written).
Ok, I've gone with `-initWithBytes:length:encoding:`… I've assumed a UTF-8 encoding… @arrbee is that a safe assumption? Or should I shoot for ASCII?
This also isn't the full story, I think because we still need to trigger the save process, right?  This is partly complicated by the fact that the save() method may need the "extra_animations" to get what the user wants.  We might be getting to the point where we need to consider having the figure object hold a list of animations that have been added to the figure (except that has a whole world of a mess in-of-itself).
There's a stray space between `(` and `'`.
This could be done better but I've run out of time.
Why did you break this (and the locator) into two pieces?
Note - there's probably more items you could check here, such as making sure that when loading the video, the write button state (pause would be seen, for example, instead of the play button). I'd either fix this later and put todos/comments on ways to extend the test or look into doing more checks.
remove the extra blank line here
It comaires two byte array if they are equal. The name should be diffrent.  I will rewrite this method
Yeah that might make more sense indeed, will remove it from here.
@miketheman I think we determined in review that this would be enabled by the LWRP now.
Merging (like mentioned above) will help reduce typos like this during maintenance.
Yes, thanks for finding this :+1:
Should be single quotes as well.
@arnodeclercq Can you add an extra line on each of the files where we get the following git error:

`\ No newline at end of file`
Yes, I'll shuffle some things around.
Would be great to consistently use just one of the `ontabsbeforeactivate` (from entwine), `this.tabs({beforeActivate: ...})` and `this.on('tabsbeforeactivate')`. They all accomplish the same, and it quickly becomes confusing to have all the variants around!

For example could this `beforeActivate` jQuery UI option be converted into an entwine event handler? Something along these lines:

```js
$('.cms-content-actions .ss-tabset.ss-ui-action-tabset').entwine({
  ontabsbeforeactivate: function(event, ui) {
    // code goes here.
  }
});
```
Expected "if (...) {\n"; found "if(...) {\n"
I think we can move this class outside the `Definition` namespace.
This does make the assumption of how the user structures their vim directory... For me ruby files are in `$HOME/.vim/ruby`
Minor : It'd be better to use map::geocode($location->location_name); here now that Geocoder::geocode_location() just calls that anyway. Then you at least get an associative array rather than indexed.
Ignore this one if you're short on time.
cval.len is unsigned, so it can't be < 0.
The output directory, usually the same as the world storage directory, goes here.
Are you sure ``$this->services`` has the correct type as ``*`` is not a sub-type of ``array``?
Is it possible to define the element type for the array of parameter ``$urlparams`` (using ``array<SomeType>``, or ``SomeType[]``)? I also found this on lines 53, and 54.
Are you sure ``$value`` has the correct type as ``object`` is not a sub-type of ``resource``?
Is it this one?
https://jira.appcelerator.org/browse/TIMOB-9980
Doch, das wär wahrscheinlich klug....
@"_def_ns" should be at least kGDataHTMLParseOptions
You don't need this if you use `contains?` because it returns `false` for `nil` as first parameter:
~~~clojure
(contains? (:awards book) award)
~~~
I think that's an invalid `<form>` element, but at the very least it's ambiguous. Mind putting in the URL explicitly? Assuming `GET` is one thing, but it's nice to be able to see the URL from the HTML source.
Ditto for next two methods.
we don't do it anywhere else, I'll bag it for now.
While it's interesting that this example works, I'm not sure it should be documented over the simpler:

``` ruby
Factory.define :user do |f|
  f.name 'Alice'
end

u = Factory :user
u.name # => "Alice"

u = Factory :user, name: 'Bob'
u.name # => "Bob"
```
This supresses a warning about DB_FH only being used once.
You should indent one less here.
Twitter accounts have name field only. Should i link it to first name, last name or split name string into two strings (but then problems occur with people with more than one name or surname)
Off course not :) Removed.
(But fix the rest!)
You can limit possible values of language with `choices`. You can limit to languages available.
`provider_sign_names` would be probably enough?
Again. This is not valid XHTML. You cannot have <ul> and text and <ul> again. Why do you want two <ul> one after another?
You cannot do it like this. This is not correct. There is a race condition here. Between checking with `get` and later on saving. This is low chance, but still a chance. And with many users, chance increases.

There are two ways to do this:
 * set username in atomic MongoDB operation in a loop until it succeeds (probably in the first try), in every loop fixing username in some way
 * saving mongoengine document in a loop until it succeeds, doing the same as above if it fails

So, in fact there is only one way: to try to store data and see if database says that everything is OK. The only problem with second approach is that it could be from some other reason that saving fails. If you can determine from the error message that it is really because of non-unique username, then this is OK.
Put `href` before `rel`?
I'm using /auth in my initial request in account views. It just gives me redirect url and if I use /auth in backends again, it demands the same parameters as in views, so this obviously doesn't work.
No problem! I'd rather more comments and questions than less even if the answer is "look down"
Ya, I don't know what happened to it all of a sudden. I'll try these changes. Thanks!
Better would to first check status, then rest.
Cool program--it needs a bit of work to wrangle it into Astropy's standards, particularly for distribution, but I look forward to seeing more of this sort of thing.
cred că poți șerge regulile css goale, le adăugăm când avem nevoie de ele.
This removes RCFileOutputFormat's protobuf dependency. expects text for columnMetadata rather than a protobuf
I see your point, and really don't  like having to think about updating header in every file I touch, but since I'm not at all affiliated with Last.fm, it really feels wrong to have "Copyright 2012 Last.fm" on files that I created.
model.__name__ ?
To aid readability, you could pre-process `lines` (when initially loaded) into an array of objects with clearer names (vs. array indices):

```javascript
var points = lines.map(function(line) {
  var components = line.split(" ");

  return {
    lat: components[0],
    long: components[1],
    elev: components[2]
  };
});
```

You could further filter out non-land points at that point:

```javascript
var points = points.filter(function(p) {
  return p.elev > 0;
});
```
nit: Stopped the Selenium Grid hub
I haven't tried this yet, but I don't think this works for requests to '/'?
I suggest you unlock the variable with the Nginx native function ngx_atomic_cmp_set(). It may be not atomic in some OS[1].

    (void) ngx_atomic_cmp_set(&lock_, 1, 0);

[1] http://stackoverflow.com/questions/8290768/is-assignment-operator-atomic
I'm in favour of  "all software development is design" as a good first-order approximation: http://www.developerdotstar.com/mag/articles/reeves_13yearslater.html
I don't think these lines match the code style of Pioneer in general...
Sounds good -- done.
There is a well-know Unix tool called `basename` which has exactly this semantic :-)
How about a test that makes sure if you freeze at 1am open_today? will still return true for a bar venue open that late, and false for 3am