labofsahil/github-event-dataset-2012 huggingface dataset:
Perhaps result[asset.site.get_coords()] is better than result[(asset.site.x, asset.site.y)]
Should we also add tests for how to handle things like `ABBRCase`?
Just because it's not the source, doesn't mean it shouldn't follow the style guide LN17-26
The same issue applies when it refers to function names: the calling of "Null" in Linregr_final::run(...).

One more place is the "trangularView<Lower>()". It needs some effort to find out where "Lower" comes from and finally confirm that "Upper" instead of "Higher" is what one needs when he wants to play with another side of the matrix :)
Keeping states is a reasonable choice, e.g., for numerical algorithms, which are not guaranteed to converge and you might want to return to a previous solution.
You forgot param tag.
This is crucial for installing in directories with spaces.
Sounds fine. If you don't want to tweak it with this pull request, create a bug to revisit it. Something along the lines of:

```
Summary: [codequality] fix LocalizingUAClient

We could merge this with LocalizingClient so that we have a client that takes a ua in the init and lets us do our testing... blah blah blah.
```
If I remove this line, the test fails.
Nit: There's an extra space at the beginning of this.
per our discussion, let's comment here about the design tradeoff of accessing app config on each call vs msg passing with one of the clients vs doing SQL generation within the client gen_server process.
oops... it's a fail..
I'm not that familiar with the internals of rapanui. I started digging only because it failed on this line specifically with rectangles.

I looked through RNFactory.createRect() and found that it specifically does not give parentGroup a value if params is nil.  I appears at least createCircle() behaves in a similar fashion as well.
this file is missing a license and a hubspot copyright notice
Definitely null this out. Deleting an object you're still going to reference is a big performance hit.
It's so ugly but I intentionally didn't do any of that here, so that this would always be exactly the same transformation from whatever shutterfly returns, and so that we do all of this kind of normalization in the map as needed (on request) and not deep in the synclet :/
These specs are blocked from running now, and fail badly when they run (the mulberry namespace is undefined). I don't know what is wrong, but I suspect it's not a quick fix.
Should add '?' and '#' to this check. If the search or hash characters are present, but there is no search or hash value, the earlier checks don't catch it.
We may want to translate this too. Currently, if you try Replace command you will see 置換 {TextToSearch} With {TextToReplace}. Or if you don't think we need this, you can just have an empty string here.
Please use `Date.toString("MMM d, h:mm tt")` which will give you the same result as using the lib you included.

Result: `Dec 2, 4:22 PM`

The `toString()` function on the `Date` prototype is put there by APF.
okay, fixed now. Wow, i'm sorry. That original line was really sloppy of me.
.main 若沒有用到也可以砍掉
Ewww. So fragile! This should look something like this:

```c
if (caps & GIT_CAP_HTTPS)
    rb_ary_push(ret_arr, CSTR2SYM("https"));

if (caps & GIT_CAP_THREADS)
    rb_ary_push(ret_arr, CSTR2SYM("threads"));
```
`l` is before `p`. ;-)
Added language field.
Don't use green please. Use something neutral, like grey without dots bellow or something. See [here](http://wlan-si.net/).
Will this plugin still work without the api key stuff tho?
This line should probably happen even if an error happens.
Should we wipe out these two constants since they aren't used anymore?
Line 1495, E:0110: Line too long (86 characters).
What does this fix?
When does the RequestMethod-inheriting object _not_ have a `headers` attribute? That shouldn't happen.
Not sure how generic the message usually is, and I think adding [id] would not hurt.
Fair enough. 0.4.0 it is.
Motivation is to force creation of users in Foreman if the user is missin (see 8841cea) and to upgrade db data form older Katello to a newer one with foreman. After discussing it with @iNecas, it would be better to move the migration to a upgrade script. Does @lzap agree?
Maybe mention in the docs why we want to filter these out? Also, why don't you just define `EBIRD-ID` in a `let` binding within this function? I'm intrigued by the design choice here ...
I think you should use ``assertInternalType`` instead to have a better failure message
As above, maybe use an ERB comment.
It's probably better to use the ``check_numpy`` function here instead of ``import numpy`` - it still throws an `ImportError`, but does so even if numpy is installed but an incompatible version.
@seancribbs @slfritchie This comment is referring to the fact that in order [kill_repairs][] to work correctly the _module_ and _partition_ data **MUST** be set before the receiver may continue.  When a repair is invoked 2 handoffs must execute.  One from each adjacent partition to the partition under repair.  Those two handoffs will result in two incoming handoff receivers.  The problem is that if you look at [receive_handoff][] you'll notice it doesn't add this data, and it can't because the information is not provided at that time.  Thus the data has to be set from the [initialization phase][hrip] of the handoff receiver.

The whole time I was writing this explanation I expected to end by saying that this needs to stay as a call.  But thinking about it more I'm not sure a cast would hurt since both the messages go to the same server and both are local calls.  So I feel like I've just muddied the waters now.

To be safe, I say leave it a call and remove the comment.


[hrip]: https://github.com/basho/riak_core/blob/master/src/riak_core_handoff_receiver.erl#L97

[kill_repairs]: https://github.com/basho/riak_core/blob/master/src/riak_core_vnode_manager.erl#L830

[receive_handoff]: https://github.com/basho/riak_core/blob/master/src/riak_core_handoff_manager.erl#L487
Sorry, you are right about that, i think we should always unset it in BaseAuthenticate if present in the userdata, not only if its in the conditions.
out of alignment with the other lines.
Remove extra space here
missing type (string)
*This place i am using vim ...  I think it might be different editer and
different bindings.*
*
*
*But , everything exposed in emacs is ok.*
*
*
*And emesene can run as expected.
*
On Thu, Aug 16, 2012 at 6:39 PM, Riccardo <notifications@github.com> wrote:

> In emesene/gui/base/ImageTheme.py:
>
> > @@ -109,6 +110,7 @@ def load_information(self, path):
> >          self.service_msn = self.get_image(path, "msn.png")
> >          self.service_facebook = self.get_image(path, "facebook.png")
> >          self.service_gtalk = self.get_image(path, "gtalk.png")
> > +	self.service_webqq = self.get_image(path, "webqq.png")
>
> 2 spaces, maybe they're 2 tabs? (translated to spaces by the text editor?)
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/emesene/emesene/pull/1460/files#r1391359>.
>
>



-- 

*Best Regards~*


*Xiang Wang*

*CDC, TrendMicro*
Right again.
Did we decide this should delete all contacts before importing the new lot?
I suspect that [commandString UTF8String] would work as well, while being more resilient to badly formatted strings.  Also, please use const char * for this since there's no reason to change the values.
I think this will fail in Python 3 - `e` only exists within the except clause, so it's a `NameError` to refer to it here. This is the sort of thing I don't like about doctests - they're quite brittle.

Ideally, we could convert this into a standard unit test - use `ip.run_cell` to define the function, `ip.run_line_magic` to call `%R` and `nt.assert_raises` to check that this bit throws a KeyError.
better to list in ApiMetadata.defaultModules, as makes it easy to override.  install() is harder to override
...Does... does Amiga have no `mmap`?
Changed to a failure.
More unnecessary whitespace changes.
An easier way of checking this would be to just check which template was used, which should be exposed in the test response under `response.templates`.
*gasp* You copied my code!
Ah yeah, I 've seen this, but that's the part after (in the "except") which made me implement this this way: we're doing some weird things with profile (creating the user from the profile for instance).

Would this (get_or_create) work in the particular case I'm handling here?
You may want a comment that all of the .CodeMirror-scrollbar* classes are only used with the flickerFix option.
This whole section seems a bit to clever for test/specs. I understand the point of trying to not repeat yourself, but it's better to avoid "coding" in specs : How are you sure that the code in your spec works ?.
I much prefer to see explicitely the expected json for items in the json.
ESPECIALLY because this is a simple case, and this bit of code it's not reused anywhere (it's not a shared example, your "targets" are always the same, therefore the expected_targets are too.
why using a different default value than the interface ?
Why ? This is a test for the XML loader
No, you don't understand it. You can pass any FormBuilderInterface but only if the parent is of the same class as the child (thus setParent can be private). So the FormBuilderInterface can still be used to implement a custom FormBuilder.
There is no need to do a count. The query as good as it gets. However if token is indeed a PK, then LIMIT 1 can be removed.
ah ... doh
no pbe here.
"Allows multiple instances of the same check"
Added libjpeg.  If someone knows exactly which Boost libs are required, I'd be happy to add that information to this change.
Seems awful close to 0.5, are you sure it isn't better to just round the aspect to the nearest integer?
** New functions end **
I'm fine with re-throwing as a HectorTransportException, I just don't see a good reason _not_ to include the original exception class name and message in the exception message.  Users don't always have client-side logging set up, and even if they do, it adds an extra step to the debugging process.
it could, why I did it like that so the usage `new_ticket(with_a_valid_requester, ExampleZendeskTicketSubclass)` read better...
Another options would be to substitue the Pdo instance in the dsn param, still specify the adapter name. Really isn't necessary since the adapter name can be detected from the Pdo instance though. It was that reason that caused me to add the null default so I would not an error on not supply that param.
Couldn't we just Ember.create the original descriptor here? @kselden
We should add a require for ember-runtime here, which I think in the minimal dependency for ember-states now.
It is because of issue JBPAPP-8517 (https://issues.jboss.org/browse/JBPAPP-8517). When it is resolved then ignore should be removed
I think this would be better if it polled for a shutdown server. I've seen times when jboss-as takes longer than 5 seconds to shutdown.
I've merged these statements. 

Check for undefined is needed as we need to call the Menu widget focus method on init. So test for undefined prevents focus from firing.
Probably no :(
You can't just toggle this here, if the parent method takes an argument.  The display could get out of sync with the value in asyncStorage.
This is consistent in all the other file (i don't like that but that's how it is done for the moment). Please keep hidden instead of invisible.
`setSeconds` doesn't take a second parameter.
OMG, sure thing.
why `<h3>`?
You did the same silly test :-/
Yeah that's yet another variable i didn't rename and he correct that for me.

(i should have merge another pull request myself for these)
This custom event shouldn't be named `select`, but you don't need it anyway if this callback is in value selector
As I noted in the CSS, I hope you can use :last-child or something so that you don't have to do this here.
nit: indent here?
Leaving those debugs statement are not so cool because you will still call JSON.stringify which is expensive.
Why is the same file in multiple projects? Can we just link it?
I'm not really testing this. The only thing we could do is make sure the commands get added to the structure and have the correct arguments passed. That said, I can't find any evidence that similar testing is done of other commands. The entire pulp_cli.initialize() method is untested.

I could add tests for that, but it'll take a little time (it wasn't as easy as I'd hoped). In the interest of making the 4pm beta build deadline, and considering that nothing else in this method is tested either, I present this PR.  But, I am happy to go back and test the pulp_initialize() method if that is desired.
definitely need a link to a useful reference for how to "make sure it's up to date"
otherwise, you just lost some fraction of your players
Should we be opening port 22? Some people might not want it open. Maybe we should just do the ports that make cassandra work? I don't know.
Possible solution : https://github.com/bkeepers/dotenv
Or use https://github.com/ddollar/foreman/
Pareil, utiliser des symboles en place de chaines de caractères pour l'homogénéité, et utiliser l'objet et non son id.
According to that page, nothing caches 302s as long as there is an expires header set to a date in the past.

Regardless, a 302 is the correct thing to be returning here.
It at least should be read from your cfg files. It defined in cfg files so it may change by chance. In this case hard code may cause some problem some day. For example now we don't start guest agent in normal case but it may needed in the future as it is very useful to get the guest status in some bad situation. Then the cfg and port name should make problem for your case. So can you think about it again?
This is a correct fix per http://twistedmatrix.com/documents/11.0.0/api/twisted.web.http.Request.html#getUser but caused some test failures.  I fixed them up.
Unrelated to your changes: This should call `previous.then` because `done` is a jQuery-specific method. The Promises/A proposal just defines `then`.
Primary transfer doesnt appear to invoke any CN matching. CN_Match option should be set to string (its not a bool) and that string should be the host name.
Is this the best way of writing the optional square brackets arguments? or should they just be moved to after each branch, since all branches now support the brackets?
ついでにここも追加しました。
Could you use `options` instead of `opts` everywhere? Thanks!
could make the directories a bit cleaner, but could also give some 
little head ache in getting all links work (back and forth).

-- 
Renaud Gaujoux
Computational Biology - University of Cape Town
South Africa


On 20/06/2012 15:42, hadley wickham wrote:
>> @@ -154,6 +193,90 @@ build_demos<- function(package, index) {
>>         file.path(package$base_path, out_path[i]))
>>     }
>>
>> -  unname(apply(cbind(out_path, title), 1, as.list))
>> +  package$demos<- list(demo=unname(apply(cbind(filename, title), 1, as.list)))
>> +
>> +  # render dedicated file
>> +  outfile<- file.path(package$base_path, '_DEMOS.html')
> If we go down this route, it would be better to have `/demos/index.html` and then put all the demos in that directory.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/hadley/staticdocs/pull/25/files#r1017618
No \n needed, the wrapper adds it
hash is freed below, so ComputeSHA1 allocs memory? In that case it can fail and a NULL check should be added.
This is my workhorse. We should try a production test before deploying it.
This pratice is unsafe. I can't give any example where this split is going to fail but it looks like something that can easily fail.
Can't we store path in module object and just retrieve it there:
Isn't main module something jetpack specific?
We may say where/when main module is going to be loaded and freezed.
It is ok, but less explicit than `URIError: Malformed Data URL: data:buggy-uri`. I let you judge your own code.
> What about using something like this:
>
>    let sandboxOptions = {
>      sandboxPrototype: options.prototype || {},
>      wantXrays: options.wantXrays ||false,
>      sandboxName: options.name
>    }
>    if (options.sandbox)
>      sandboxOptions.sameGroupAs = sandbox;
> 

I wish I could! you will get warnings if options don't have those properties.
Maybe we should have a function for that.
I would rename `getPrefKeys` to just `keys`
I'm fine with either of these options. I'd probably suggest something like:

```js
// Maybe just define some private name in module
var name = Math.random().toString(32).substr(2)

// And use that name to define internal properties
return Object.defineProperty({}, name, {
  value: nsIZipWriter
})
```
@ozten - for proxyidp's, the type was originally "proxyidp" and this differentiation allowed the front end to differentiate between a BigTent primary and a normal primary.  BigTent primaries were not shown the "you have to verify with your primary" with the URL screen.  Is the change to "primary" intentional or should this still be "proxyidp"?
Along with this rebrand, now we're ready to localize the main site, eh?

Shall we open up or revive a bug to go through and wrap strings post-merge?
it would be nice to short-circuit this logic even sooner, because it seems like there's a lot of work being done on the router that isn't strictly necessary.

How much code would we remove from request processing if we were to implement something like a `wsapi.routeMap()` function that returned a map of read URLs and write URLs?

Then I wonder about when we start implementing anti dos stuff.  What's the best code organization?  Would it be nice to be able to write checks, then associate them with some number of api calls?  will all of our request checks run for all apis?

hmmm.
I am ashamed for not catchign this!
nit: mind adding a newline? Also please rebase afterward to a single commit for checkin.
It's executed with `call_user_func` and thus takes all forms of callable, like anonymous functions, methods (PN or array notations) etc.
I think it is ok to depend on `Facter::Util::Resolution.search_paths`. Note that seach_paths will always append `/sbin` and `/usr/sbin` to `$PATH`. Otherwise facter would break when run as a non-root user because it is uncommon to have `/sbin` in $PATH as a non-root user so `/sbin/ifconfig` may not be found when just running `%x{ifconfig}`
There is extra whitespace here. We try to not have trailing whitespace on things.
That made me curious, so I looked at the tree, and ended up with #1103 that implements my suggestion.  It should merge to 3.x soon, so you can expect to end up dropping ScopeDecorator when that happens.
The lulz
Your method is named `inject_attribute_to_fog_model` -- but that's just a **side-effect** of the method's REAL work, which is to turn the Fog model into a Hash (augmented by some extra attributes), which you then turn to JSON.

I suggest you rewrite the private method as follows:


    # Returns a Hash of a Fog Model (parsed from to_json())
    # Any extra_attributes are merged into the reuslt.
    def fog_model_to_hash(model, extra_attributes = {})
      JSON.parse(model.to_json).merge(extra_attributes)
    end

It can be invoked in the same way as you are now, but this version is shorter, clearer, and merges any number of extra attributes, not just the first.
Ouch. I'll definitely fix this and also try to cover it with a spec.
do span3 and 9 add up ? i remember you mentioning something about that; that it needed to be one off.
I appreciate your pedantry :+1:
Fixed. We log now all warnings in one line
you want create from list of strings one string, so for me right function is reduce - http://doc.opensuse.org/projects/YaST/SLES11/tdg/list/reduce-external.html
Of course, this should be the case, but curently this is not completed in the whole framework. So I would leave that for an extra hotfix branch.
Use Zend\Stdlib\ErrorHandler if you need to "silence" errors. The usage is:
```
ErrorHandler::start(E_WARNING); // or any mask of error constants
// work that may raise errors here
ErrorHandler::stop();
// check for conditions that indicate an error. Alternately, pass a boolean true value to stop(), and it will raise an error
```
Error suppression should not be used in the framework, as it has the potential to hide critical debugging information.
Please make sure that you are able to use the Calendar package in GWT.
Not a critical format error I assume ...
Hmm, when this is updated for entities, have difficulty affect entities too? Just Players? Situational(entities damage increases with difficulty but only that)
This could become an Attribute of the app (that's make it more YUI-like).
Not inherently. What if the user just prefers RSS to atom?
Its my erlang background, but I would prefer to not modify and reuse variables from their original intent, granted 'newResults' was a terrible name. 

How about var dedupResults = [];

and use that after this block instead of 'results'
wrong error message. Nothing is passed to move but to reorder
i think this should come right next to setReferenceable, as it extends referenceable. it is inconsistent because for referenceable, we have no if but always set. i think this must follow the same logic as referenceable.
Don't think so. `require_user` is the function I use when a link should redirect you to the login screen - the only situation is submitting an article when you're not logged in.
Couldn't we make this simpler? I don't like much that we need to pass the constructor around. Even more, I dislike we need to pass the constructor around for every iteration, when we probably don't need them.

I believe a better way to solve this problem is to have a way to retrieve the protocol and its collections. So we could do something like:

    Enum.Iterator.__apply__(:iterator, [list])

That would return both the result of the invocation and the implementation module it was invoked on, allowing us to access the constructor straight from the source if we need to.

Wdyt?
CS : Unused line
already in copyright header
Whitespace out here.
Found N + 1 problem in documentation, still not sure I understand it: http://guides.rubyonrails.org/active_record_querying.html
I think this should always set parser->type to HTTP_REQUEST, and set parser->method to HTTP_HEAD or HTTP_GENERIC depending on ch.
Probably don't need 255 max_length, but it's also probably no big deal.
Not sure what you mean here. Those builds are release candidate builds. Skipping release here will be misleading.
group the intializations before the init + assignment, like:

```js
var eventInterface, evt, clonedEvents, 
       events = this.data.events[ type ];
```
No more explicit parameter dictionaries from YML files.
yep that's right, doesn't work :P
Maybe you can just let it throw instead?
I don't have any specific opinion on that.
This patch as it conflict with Greg previous PR. I don't know if foreman will support one day former Rackspace Cloud Server.
I don't know how long Rackspace will support .
On the other hand I don't see any problem to specify V2 version.
You don't need these steps at all.

on the command line use `gaiatest --testvars=testvars.json` and it will be imported automatically into self.testvars object. Sorry I musn't have explained that in my earlier comments! sorry
It is a matter of opinion, I suppose. However, these IDE files are autogenerated by Eclipse, and may cause edit wars and unnecessary commits if not ignored.
Shame is too harsh, I dare to bet that there’s code in the standard lib that uses this pattern :)
If this removal is needed, please make it a separate commit and justify it.
Ok thanks. I'll use this on the branch that will use this version of the Sip-Creator.
Why would you use camel case here? http://www.python.org/dev/peps/pep-0008/#function-names
if JavaScript is disabled, the user can not configure the workflow event anymore.
It would be better to adopt a progressive enhancement strategy by hiding the button with JavaScript.
Actually, it would be even better to either execute the on change binding and the hiding function in an anonymous function (after the select) or when the document is ready.
eZDebug::writeError() instead ?
There is one more of this in ezdbclusterstalecache_test.php
Can you add a blank line before `foreach`?
PEP8 likes a new line at the end of the file
Any reason for not using Pprz.alt_unit_coef_of_xml here ?
Renamed to `hashAndComparePassword`, and changed the variable naming to be a bit clearer. Let me know if that suffices.
Please make the error message a bit more descriptive.
user is a bit deceiving. memberships is probably better.
I think we can just pass req.body into updateGroup and have that dal with invalid profile fields (which I think it does already)
Please, yes. Also, you'll have to pull, because there will be conflicts.
Yes, it does. I don't think the "more descriptive class [to] allow the separate styling" is necessary.
Good catch. Not sure why I have this in here. Removed in 804c425.
Use BSD/Allman bracketing - http://kohanaframework.org/3.0/guide/kohana/conventions#brackets
The whole code is based in the com.sleepycat.je.util.DbSpace, so some of the command-line arguments came with the code.  I've modified the class to accept it as a parameter, because I think can there may be cases were may be needed to recalculate the file summaries
no need to close this tag
I think this should refer to a local variable dirty. Changing this.dirty on line 72 will not update this variable..
Yes it's true something like `The GeoJson dumper`. But I was wondering if we should follow the sf2 coding standard 100% ?

Maybe you can make a PR about sf2 coding standard so I close this one :) What do you think ?
While doing the rest - might as well remove this '// '. :)
Теоретически self может вернуться ошибочным, в этом случае нет смысла инициализировать объект
Согласен, поправил код в новом коммите
неизвестно через сколько времени закончится цикл. Лучше в массиве последовательных по возрастанию чисел (0...99) менять случайные элементы местами, например 100 раз.
This line is ridiculously long. Please break up.
I believe this needs to be `extraOptions.jsonpCallback`
To make sure that the subscriber has the update saved, otherwise, we could imagine a scenario where the create and update could get merged because of deferring.
Missing a space between `if` and the opening parenthesis. There's no need to assign `$targetClass` here, especially since its only used once on a short line.
I'll make the modifications...

On 4/19/12 12:17 PM, "doubleotoo"
<reply+p-709121-c71dd0e7c832eaa33e75300a58d233c39a4c2f70-1088219@reply.gith
ub.com> wrote:

>> @@ -1,2 +1,6 @@
>>  Testing GitHub Wikis.
>>  
>> +
>> +I am editing to make a pull request
>
>I don't know about this... we shouldn't wrap the line so quickly.
>
>--- 
>Reply to this email directly or view it on GitHub:
>https://github.com/doubleotoo/test-wiki/pull/3/files#r709121
Thanks for the inspiration. Ok, after rethinking it "close" is the closest explaination - hm, sounds logical :smile:.
we should have compute_proj_exg so we can factor the code for EOG.

we should also have an mne_compute_proj_eog.py as for ecg
Ahh, now I see what you mean...
I would allow score_func to be a string that default to this lambda when equals to "corr"

it much faster to remember and it would be reasonable default
legacy, outdated

On 19.10.2012, at 21:10, Martin Luessi <notifications@github.com> wrote:

> In mne/fiff/raw.py:
> 
> > +    data : instance of numpy.core.ndarray
> > +        processed data matching the data contained by raw
> > +
> > +    Attributes
> > +    ----------
> > +        See __doc__ of mne.fiff.Raw
> > +    """
> > +    def __init__(self, raw, data, picks=None, info=None):
> > +
> > +        print 'Initializing raw object from merge with custom data.'
> > +
> > +        if not raw._preloaded:
> > +            raw._preload_data(True)
> > +            raw._preloaded = True
> > +
> > +        raw = deepcopy(raw)
> why are you not using raw.copy() for this?
> 
> —
> Reply to this email directly or view it on GitHub.
>
because you've put "Save this" into the title field so it finds it on the form when the Preview button fails.. right?
This is not being used anywhere in the prompts.
Is this change needed? The pattern named `profile` will only exist if `ACCOUNTS_PROFILE_VIEWS_ENABLED` is `True`, and then by using the `url` tag with an `as` arg, the variable we bind to will be `None` if the pattern doesn't exist, which is why it's checked for underneath already.
直到匹配到第一条规则。用主动
Yes, this probably should have been in its own commit -- its a patch to my last commit. I somehow missed it in the last commit for generators. It is used here: https://github.com/thoughtbot/clearance/blob/master/lib/generators/clearance/views/views_generator.rb#L6-6
I meant you can use `Collection.find(func)` here, not the underscore function. They're all proxied into the Collection.
It appears not.  I'm not completely sure what's going on inside rabbit, but this case gets hit during various recovery scenarios.  Basically the method checks to see if the channel actor already has a channel defined, and if so, it closes the one sent in the message if it is open.  I'm not sure I want to tamper with this since it appears to work.  I would like to get a better understanding of it at some point.
Why this catch block?
why map?
А зачем эта структура в хедере?
`rowItem`
Just porting the simplification from cucumber-vimscript.
Okay, understand.
Ah, cool.  However, I still need the nil check, so I'll just change the call to "replace" to a call to "name".
...FileName maybe? as this returns a name, not a file. Otherwise one may be confused by the code in the settings activity, where we have something like new File(get...File()).
Tämän ja seuraavan tapauksen voisi yhdistää käyttämällä `get` funktion kolmatta parametriä, joka palautetaan jos avainta ei löydy hajautustaulusta:
~~~clojure
(assoc freqs
  (first a-seq) (inc (get freqs (first a-seq) 0)))
~~~
out is being closed but not in.
That's the way it is right now, but I thought we were talking about future proofing?  In the future, someone might add attrs to return for the text part of the text object, and at that point, if you don't merge the two dicts, it gets lost.

The text object is made up (possibly) of two parts...the text and the bbox_patch.  The contains test should return the union of those two results.  Union of booleans, then union of the attrs.
@dmcdougall Why *would* you want to use cbook.is_numlike or is_scalar here?  In any case, it looks like a change to the line in question has already been made by another PR.  

@mwelter, I don't think this PR should be touchiung backend_pdf at all.
This change is not clear at all, and should be commented.

I'm guessing you want to avoid  the case were ``ret`` is an empty list, or an integer equal to 0 etc... ie, all the case were ``ret`` isn't a boolean, but evaluates to false.

That method is tricky and smelly: I wouldn't expect that in a python code.
I love that we're selecting the report by an explicit type!
we could use here top_crashers[0]
we could remove some duplication
Shouldn't this be 

```ruby 
z = t.match(/^pub\s+\d+\w\/([0-9A-F]{8})/)
```
remove the whitespace at the end of this line, please?
rl-version --> rightlink-version
Can we consolidate as much of this as possible into one function?
Where dit this formular come from? I'm just interested in the logic behind doing it this way.
why do you need static_cast here? isn't int32_t(x) sufficient?
PYPROTO_DENSEFEATURES?
=== rather than ==?
I ended up making this change in 8b4c6c6 so that we could test with it in place.
Report dates can be in the future - and were in my test dataset - therefore export dates need to be able to be in the future.

I missed that it was also checking to make sure both dates were selected.. any reason we can't allow just 1 date?
ie. if I only select from_date = 1 June 2012, then give me everything after that date.. no upper limit?
If we can't do that then I'll add that check back.
I suggest adding .css('pointer-events', 'none') before .fadeOut() to prevent users from clicking buttons/links inside post while post disappears and possibly triggering js errors. There will always be some users who would try clicking disappearing post to see what happens.

Same for all other .fadeOut() entries in this file.
In general this looks fine.

I question whether an MS should be a strict requirement mostly because of
candidate(s) I know who would be ideal for the position save that fact. I
would almost place experience in team/model development higher than the
degree requirement.

Matthew Gidden
Nuclear Energy University Programs Fellow
The University of Wisconsin -- Madison
Ph. 225.892.3192
Fax 608.263.7451



On Thu, Nov 8, 2012 at 11:10 AM, Julie C. Zachman
<notifications@github.com>wrote:

> In source/people/intern-2012-cyclus.rst:
>
> > @@ -0,0 +1,64 @@
> > +Research Intern Opportunity (2012): Agent-based Fuel Cycle Modeling
> > +=====================
> > +
> > +*[Updated: November 4, 2012]*
> > +
> > +The Computational Nuclear Engineering Research Group (CNERG) is
> > +leading the development of a novel open-source agent-based platform
> > +for the simulation of advanced nuclear fuel cycles, Cyclus
> > +(cyclus.github.com).  The core infrastructure of this effort combines
> > +cocnepts from operations research, computer science and nuclear
>
> cocnepts -> concepts
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/CNERG/cnerg.github.com/pull/2/files#r2072413>.
>
>
Why do you do this?
- Are you sure ``null`` has the correct type as ``null`` is not a sub-type of ``object``? Please also check line 52.
- Are you sure you can call ``setValue()`` with the given parameters? It seems like at least the 2. parameter (``$value``) is missing, no? Please also check line 52.
Should the return type not be "type" instead of "string"?
See https://github.com/brikteknologier/seraph/pull/59/files#L3R174 for how the non-streaming version handles the same thing
Can you make this 80 chars?
ah, guards
you could have an error here
simplexml_load_file produces an E_WARNING error message for each error found in the XML data.
there isn't a `getGetter` function yet.
Why is there a `main()` method? shouldn't that be removed?
This needs to be an array by default, otherwise it will be `nil` [here](https://github.com/presidentbeef/brakeman/pull/73/files#L4R503)
You have not specified the string's encoding. The platform default varies between platforms.

Recommend explicitly specifying UTF-8 encoding.
@mnoble - I modified this to raise an ArgumentException when the argument is not a Numeric or UnitValue, and removed the rescue.  Interestingly enough though, the Comparable implementation of ''==' rescues exceptions and returns false.
Sorry. I was not clear enough. Structure is perfect here. So dict is a good choice. I had problems only with name. So `panels` name is not good enough. and later on in the code you need three levels of elements (it is OK to need three level of elements), so how would we name the top element, so that lower elements would also have a good name. So if you start here with `dependencies`, how you iterate over its nested elements in a way that it is understandable. I proposed `panels_dependencies` -> `panel_dependencies` -> `panel_dependency`, but I find it ugly.

(The other minor thing was to use `context.update`.)
Unnecessary space at the end of the line.
I think i made a merge with the other branches. Which might be a bad idea. Maybe we should stick with merging only with master?
What do you mean "How do I remove it?" Doesn't it work after just deleting it? or what?
These directory paths should be constants.
It just makes the select list with the pre-populated names, the type of
list that you can hold ctrl and select multiple items.

On Sat, Nov 17, 2012 at 1:32 PM, Conor Reisman <notifications@github.com>wrote:

> In fullhouse/templates/dashboard.html:
>
> > +					</p>
> > +					<p>
> > +						<label form="time"/>
> > +						Time: <input type="time" name="timedue" class="time-due"/>
> > +					<p>
> > +						<label form="assigned"/>
> > +						<p>Members: </p>
> > +						<select multiple="multiple" name="members" class="member-list">
> > +							<option value="1">Janette</option>
> > +							<option value="2">Conor</option>
> > +							<option value="3">Brandon</option>
> > +							<option value="4">Jeff</option>
> > +							<option value="5">Cornel</option>
> > +							<option value="6">Shahaf</option>
> > +							<option value="7">Nat</option>
> > +							<!-- TODO: Implement members, above are just placeholders
>
> What does this code do if run?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/Full-House-UW/fullhouse/pull/121/files#r2160285>.
>
>
Same as comments for the other `find-` related factories
@jturnbull do you mean this change when you say "new objects go on the end rather than the beginning"?

This didn't actually make it insert at the beginning because of lines 70-75 (before diff).
Maybe fix some spaces here too? Nitpicking...well :-)
So, this is the complimentary point wrt a change the Kiyan suggested. Previously I had been passing around a metadata record builder, and recording the relevant info from the meta + schema tables. Here I have replaced that with method in KijiMetaTable that is dealing with the whole record and passing info around to the schema table. I think I'll have a restoreMetaData method in MetadataRestorer, and then have methods for the schema and meta tables that are fromBackup and toBackup.
it is probably a good idea to validate that $parsed is a list,  $class is a valid class. Validating the existence of the create method might also be possible via Reflection.
We don't usually line up assignments.
Maybe this should be done on the entry, once a responsebody is set. Making a `new Image()` per image resource whenever you call `get_resources` seems a bit wasteful, this is probably called a lot.
We need to keep the 'service[nginx]' syntax.

The timing can be removed, though.
newline not needed
To minimise copying of vectors, this should be:

    const std::vector<Light> &lights = camera->GetLights();
    std::vector<Light> newLights = lights;
I cannot really tell from the diff, but did you add this checkbox over the "Use UPnP / NAT-PMP port forwarding from my router" one in the "Connection" tab? I think this would be its right place.
re: ``os.rename`` vs ``shutil.move``
-----------------------------------------------

I would guess that anything in ``os`` is a sufficiently safer bet. I think shutil may be OSX/Linux specific but I'm not entirely sure. All I **am** sure of is that in my scripts, working with the ``os`` module works across OSX, Linux and Windows.
Yes, tight_layout() makes the plot looks much better, but it only works for matplotlib > 1.1.1. I added this just before the return call in `plot_partregress_ax`:

```
try:
    ax.tight_layout()
except:
    pass
```

The results looks great locally (though I still hate the default theme :P ).
better start_params ?
open a new issue and check this later
Okay. This snuck in there. This was done because I had restores fail for various reasons and then it would bomb here.
Why border attribute? This should be done through CSS.
*Use SSL for default gravatar URL, or not*
Same comment as above re:ImmutableMap
I just made an attempt to merge in a slightly modified version of the para from draw that you mentioned (I didn't exactly keep the first sentence of this paragraph as is, instead I took the paragraph from the ```draw``` dox and modified it appropriately).  I also updated both the draw and drawShoot version to say the names of the keywords it's referring to.  What do you think?
Doesn't this need to be protected?  Add comments to public property.
Ah, yeah, the rule I just said is incorrect. Nothing in the LMS -> LRS launch doc says you can't send back reserved params. But the behavior here is also incorrect... 

Is this from the thing where posting with a registration caused a statement query fetch? (If so, I think the right fix may actually be on the server side, to check POST request bodies for posted statements first, and favor that, instead of looking for query params and thinking it's a GET, regardless of if there's any payload POSTed...)
I'm not convinced that Iris *is* a "framework". I would have said it was more of a toolkit, but even that I am not 100% sure on...
yup agree
Why is this clearfixed?
Abstract out into method
Do we also want to do the "waitUntilInterrupted", if stopOnKeyPress? Or should that be in an else block?
Please use mock instead of fudge to mock and test the call for this, no point in having 2 mocking libraries.

I can also handle this next week if you can't find time to make the change.
Doesn't seem like something they're likely to do, but I've added a check for it.
Also I just realized we can make reportCreated() no longer take a listeningroom_ptr, since the listening room itself can now do m_weakRef.toStrongRef() to get the sharedptr.
this file should be probably moved out
ouch, yes, that is a different story altogether
you probably don't need |== true| here.
ALFIECODE
Am sa adaug
Let's change the method signature here from taking a Job to a StepLocator.  As it is right now, the method asks for one thing (a Job implementation) but really wants something else (a StepLocator).  Why not have the method ask for what it needs (the StepLocator) and have the caller be responsible for meeting the contract?
I'd probably add more to the safe_id, like some kind of cached_region_ prefix or whatever to reduce the risk of a region by coincidence having a name of an existing instance variable
Название пакета начинается со строчной буквы.
Зачем здесь преобразование к строке, а потом обратное преобразование к Integer?
1.0.1.Final already released
PEP8 issue: reviews.py:19:25: E225 missing whitespace around operator
全てにヒットします -> 全てが該当します。全てが実行されます。

ヒットはちょっと違和感ありました。
If we want to keep the generic WallPost API, this should be fixed.
Suggestion: My localhost tripped on this line because the file wasn't there. I worked around it wit a try: but we could do a proper file-exists check. Or do you think the file shoul always be there?
make sanitize private
It's likely that the fixtures will be needed to be updated because of this change. I can take care of that in my #16 branch.
Could you explain this?
``len(conntected_components)`` is always the number of nodes (=``graph.shape[0]``), right?
So you break when all nodes have been visited?
Why not do that right after ``last_num_components`` was assigned?
Yeah I changed that but didn't push last night for some reason ;)
Yeah so if people use the non-deprecated code, the code below will do the right thing. This is testing that users using the deprecated interface are not able to mess it up.
This client needs to be closed in a finally block or some other way.  Since whoever creates this security domain should also *always* remove it when done with it, I suggest that that caller takes responsibility for creating and closing the ModelControllerClient. Typically this is done via @BeforeClass and @AfterClass methods.
well that's just javascript, I dont want to make it not javascript. you can do `#{foo.bar || 'baz'}` or if for example `foo: undefined` was passed to the template because maybe the query returned undefined, then `#{foo || 'bar'}` works just fine
This does not need to be in a function.
The name should come from envData.results_dir rather than being hard coded.
Removed in both cases.
Add validation for filters (each key must be a string)
Another inconsistent label...

What does 'mods' mean anyway?
It could be - you would gain some extra error checking I guess (you would detect truncated packets).  Not sure it is worth it.
This layout change was requested by operations.
I prefer to add features on demand, than to predict what others will need... I might need it on the next iteration, when I'll then move the code into the header file and use it... but not before.
Why is this method being added to `SharingSCMTest`? Most test cases should be in `MercurialSCMTest` so that they are run in this suite, i.e. in basic mode, as well as in the several subclass suites representing alternative configurations of the `MercurialInstallation`. Moving it to the normal place would also allow `repo` to remain private.

(Thought I had pointed this out before, but perhaps GH swallowed my comment?)
In fact, I'm pretty certain we should not do this. Please revert this to the .min. file.
Okay - if we don't know why the code is there, and the tests pass, let's remove it. I don't want to have dead code slowing us down.
You can just use `File.read` here, and it is generally speaking better to handle this by trying to read optimistically and catching the exception - because you can totally still get a failure trying to read a file that didn't exist here, if someone races to unlink it.
Sorry, more comments! `value` (which should be probably named something like `query`), can only be a String, Integer, or Range. Otherwise your implementation in this method is fine.
A class definition like
cf2_if_else("password_setted$(usergroupuser_index)", "password_failed_$(usergroup_user_index)")
would be welcomed
How about `if isinstance(self.table_pagination, bool):`?
would you please parameterize template option 1. the number doesn't mean anything to me.
@shacker It's good that we're doing this log message but can we also add where it's coming from & also the courses failure?

```
tasks.js -> Could not load assignments or courses data from Canvas
```
Blanks section needed? Or is this a placeholder for future event handlers?
Only the last port_translated is being used, right? If there is a need to report all the port_translated found it is wrong. If you only need to report the last port_translated found this loop isn't needed, which would do code more readable.
This is already created by including Auxiliary::Scanner (and even if it were not, it should be an ```OptAddressRange``` instead of a string)
I keep getting this error at this line (155):
#<Rex::Post::Meterpreter::RequestError: stdapi_railgun_memread: Operation failed: The parameter is incorrect.>
Shouldn't this just be removed and not just commented out?
I think it's better if you do this, because you know your desired conventions better. (hopefully at least ;-))
``super_create`` seems overkill vs. just passing the kwargs to ``create`` above.

If you were finely crafting the arguments you're passing to create and it broke 80 chars, splitting it up like this would make an excellent pattern.
I assume you mean `->getIterator()`, not `->getPaginator()`?
"Please try another." (It's a full sentence.)
If you just want a random string, the jsonp class has something simpler: `Math.random().toString().substring(2, 8)`
Was `ArcGISTileProvider` removed or replaced by `ArcGisMapServerImageryProvider`?  If so, we should update CHANGES.md.
You should use path.join() instead of directly concatenating directories with a '/'. This will future proof it for Windows support.
Style issue; unless they're used outside of loops, `for` iterators should be declared in the loop.
Needs a period at the end. :) Sorry, but we have an advanced mode in validate.py that checks for that.
Could possibly make the widget.js scheme-relative and use `//widgets.twing.com/j/2/widget.js` once https://hannonhill.jira.com/browse/CSI-404 has been implemented.
Where are these attributes specified? Is that oldschool html? html5?
Ok I will remove it
хотя может и не надо убирать
$result is never used. I'm guessing this is supposed to be `$this->discovered = $this->discover();`.
I might be missing something, but cannot you use ScalaCompilerPreferenceInitializer#initializeDefaultPreferences() to set the default values?
I think that would do it at the workspace level, then being copied to each project when selecting "Use project settings" (at least its how it should work)
This is extra work (initialization followed by assignment). Better: initialize with void and then use emplace.
This is already mentioned, and I must say I didn't like the wording on this one the first time around.
LGTM
__FILE__ and line, ehm?
Example custom overlay use in this method
Should be `\s`, yes. But if there's an easy way to access the list of libraries directly from `toolchain`, there's no need for this.