labofsahil/github-event-dataset-2012 huggingface dataset:
My thinking was like below.

Both of `checkingRequiredSettings` method and `getRequiredPropertyValues` which is called by `checkingRequiredSettings` methods instantiate a new `Map` to maintain the required properties.  With this statement, `synchronized` keyword is not necessary even though multiple `RedisDatastoreProvider` instances call the method about the same time.

 However I thought that there would be some way to change the properties' values while other `RedisDatastoreProvider` instantiates are about to get the properties. If exists, it would be better to make the method `synchronized` to maintain the consistent state. If this process is read-only and there are no other ways to modify the properties' values, then `synchronized` keyword is not necessary. But I was not quite sure if it's read only or not.
Toss in some comment to explain this file and it's purpose, if any.
Shouldn't sudo be required from EL5 so that non-root iptables rules changes work also? This would fix the root cause of the issue for el5 (access right problems).
cb (if err then filePath else stdout)
Perhaps an initial oversight, should we rename this submodule?
Good point of separating that in other class that also will make CCM looks light when we start adding more default sources. Will fix.
It does, but then it would have a dependency on that. This class is now stand-alone from any other Dragonfly code, so it can be reused without dependencies.
Nope, I didn't realize they took different arguments. I think it's fine as it is.
We should also probably make sure the port QLineEdit is disabled when this checkbox is set. This would be less confusing to the user.
That sounds good. I'll do that and re-push. Maybe I can just comment these lines out so people who look at source can get an idea what content is still missing but they won't clog up the site.
doc for args? What's the main entry-point for users ?
Now we have a `vimStateEnabled` and a `VIM_ENABLE` variables. Quite confusing.
Should there be an underscore prefix on the `CppHSMShapeData` referred to on this line?  There is one used in they `TypeError` exception message just below, and I think that's right, but I just want to check!
Erm, this is weird.  When I leave ```E_power_function``` the same but make B=0, I'm getting var(g1) = 0.0043 and var(g2)=0.0057.  Likewise if E=0 and ```B_power_function``` is the same flat one as usual, I'm getting var(g1)=0.0057 and var(g2)=0.0043.  and of course 0.0043+0.0057=0.01.  But this wasn't what I expected and I'm not sure where the 0.0043 vs. 0.0057 comes from.
fixed, checking properly for all others... (by eye currently...!)
I'm surprised you touched any of the chef stuff -- did you test this?
no noticeable different.  have added comment about why we prefer kill -HUP
If it times out, the exception we throw will just say that it timed out and the URL we were trying to access. The thrown exception won't include a stacktrace of what the connect-task was doing. That exception will only be logged at debug (when the connect task catches its InterruptedException).
This one I don't like. Using parens to invoke this when no other function is invoked by parens bothers me, but less than using a keyword to invoke the function does. 

Function application in almost every language happens from the right - you name the function and then call it (possibly with parameters). Flipping that here seems unnecessarily obfuscating. Also I imagine this doesn't play out too cleanly in other situations. For a relatively tame example, which do you prefer between:
`if @animate() == true` and `if (do @animate) == true`
*minutes
I had forgotten about the newline nemesis that git has. I have added this in my local repo - maybe after the code review comments, I could correct this..
Nice! Looks great. Jim

On 4/26/2012 4:19 AM, Zsolt Berentey wrote:
>>   </p>]]></comment>
>>   		<author><![CDATA[Zsolt Berentey]]></author>
>>   		<author><![CDATA[Shuyang Zhou]]></author>
>>   		<method>
>>   			<name>addActivityCounter</name>
>> -			<comment><![CDATA[Adds an activity counter into the database.
>> +			<comment><![CDATA[Adds an activity counter with a default period length.
>>
>>   <p>
>>   This method uses the lock service to guard against multiple threads trying to insert the same counter because this service is called asynchronously from the social activity service, this method must , so it is using the lock service for that.
> Yes, thanks for catching it. I rephrased that sentence and left that part in :)
>
> The correct sentece is just this:
>>> This method uses the lock service to guard against multiple threads trying to insert the same counter because this service is called asynchronously from the social activity service.
>
> ---------------------
> Zsolt
>
>
>
> On 2012. Apr. 26, at 3:36 AM, James Ross Hinkey wrote:
>
>>> </p>]]></comment>
>>> 		<author><![CDATA[Zsolt Berentey]]></author>
>>> 		<author><![CDATA[Shuyang Zhou]]></author>
>>> 		<method>
>>> 			<name>addActivityCounter</name>
>>> -			<comment><![CDATA[Adds an activity counter into the database.
>>> +			<comment><![CDATA[Adds an activity counter with a default period length.
>>>
>>> <p>
>>> This method uses the lock service to guard against multiple threads trying to insert the same counter because this service is called asynchronously from the social activity service, this method must , so it is using the lock service for that.
>> @zberentey Hi Zsolt, please clarify what is intended by the part of the description that states "..., this method must , ...". It seems as though some text may be missing. thanks.
>>
>> ---
>> Reply to this email directly or view it on GitHub:
>> https://github.com/zberentey/liferay-portal/pull/55/files#r736579
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/zberentey/liferay-portal/pull/55/files#r737488
Oops, I just copy pasted from the forum thread
I'm completely ok with this but would like to leave that to a separate review.  I'd like to get the simple navigation in + the refactoring of the pathplanner. 

There is a lot of work beyond this to make it great IMO but I suspect the "correct" solution will involve removing any navigation specific code from manual.  There are so many lines of code in there it would be a LOT of duplication just to avoid 20 lines that are #ifdef'd.
Here you might an argument from Stac.  He's always preferred that the code be standalone complete to bring up a system.  Mike Smith, who wrote most of the F4 code, preferred starting all the clocks at the beginning because the difference in current was tiny.  I'm fine with this though.
Didnt get the purpose of this also, Gouge spell effect [1] is Effect 1: Id 80 (SPELL_EFFECT_ADD_COMBO_POINTS), And why are you preventing the spell to proc if its own damage hits the target? sounds kinda nonsense.
not very obvious to me. Where's the opening tag?
Такие комментарии надо вычищать.
Экранировать надо содержимое args
missing whitespace after ' : '
This should go in core_ext.rb
"OpenShift login id (Red Hat Network or OpenShift)"
I'm sure this variant is better for performance. I don't feel like ```.html_safe``` will make code much more readable.
Skip the `len()` check, not necesary.
typo in neighours
the active set list should be a sorted list see eg. to maintain a list sorted in python:

http://www.doughellmann.com/PyMOTW/bisect/
There's no shame at all in copying code from the project for other parts of the project :)
you should document the attributes obtained after fitting

Attributes
--------------
loglike_ : ...
    ...
Wouldn't it be possible to do ``np.exp(scores)`` instead?
add `lobpcg` to valueerror
it will always be set as a prototyped array node defaults to an empty array. So setting the parameter can be done by ``$container->setParameter('swiftmailer.delivery_whitelist', $config['delivery_whitelist']);`` for all cases.

And I don't think it makes sense to enable the plugin if the delivery_address is not set but only the whitelist (I even think that it may break things when sending a mail to a non-whitelisted message).
Pas de `begin` /  `commit` /  `rollback` tout ça tout ça ? Ca pourrait jouer sur les perfs non ?
If I changed this to `"question.selectedAnswer"` would it work? I'm not sure what context that will get evaluated but I hate to reach back out into the global scope when `question` is available locally.
This seems like yet another instance of the semantic/resolveProperties/ctfeInterpret triple – maybe adding a helper for that is in order, so that resolving compile-time constants becomes less error-prone?
Sure, I just added the line for the comment.
For clearInterval and clearTimeout functions, platforms are not mentioned.
Curious. Why are we setting the container's background and not the control? Maybe the container should always be transparent and we should only modify the colours for the controls?
I don't think this is available in the r7 support library (the only one available through Maven)
Si le fichier ne correspond pas aux caractéristiques des :doc:`services des douanes <../../../../Admin/Configuration>`, il est placé en quarantaine. [vérifier le lien suggéré]
I think I've got some working code, stand by.
A paginator is a hard dependency for this object, cannot be null, please remove the `= null` default value and related logic
Make this `return $this;` as well (and update the docblock).
I'm divided between creating a specific flag for hash_dedup or specific for non-deterministic results... For now, there isn't much difference...
possibly clearer if moved after the if, and if(!extractKey) becomes an else
it could... not necessarily should... What's the general guidelines for inlined public functions in HPCC?
this is not needed since SocorroMiddleware will insert that automatically.
this works fine. the variables can be appended to the current command as arguments. we can talk more on IRC

On Sep 19, 2012, at 4:16 PM, bradallred@me.com wrote:

> That sounds possible, but im not sure if the build environment can inherit variables that way. I will test.
> 
> On Sep 19, 2012, at 4:13 PM, Tom Prince <notifications@github.com> wrote:
> 
>> In master.cfg:
>> 
>> > @@ -291,9 +291,10 @@ c['builders'].append({
>> >      'factory': BuildFactory([
>> >          gitCheckout(),
>> >          gitDescribe(),
>> > +	ShellCommand( name="link iOS libs", description="add sym link to iOS static lib directory", command=['ln', '-s', 'lib', 'build/apple/ios/lib']),
>> Can you just make the library location an environment variable, that can be set externally?
>> 
>> —
>> Reply to this email directly or view it on GitHub.
>> 
>> 
>
u already fixed that, u can ignore my comment in SB PR
Non, ça cache juste le message au dessus du formulaire qui dit "Votre formulaire contient des erreurs, blabla".
Mais ce n'est pas nécessaire de le laisser car chacun des champs montre déjà qu'il y a des erreurs, avec le message explicatif.
Do not review this page
How do you know this one is going to be always the first one. We want to make sure we can run against different users and different environments as much as possible
@raydavis That's fine (and good) - I'll change the emitter references in tasks and notifications.
Yea that's fair enough. Let's keep it simple and one file for now.
I would simply use ``RuntimeException`` here, especially as it is used in the DI extension, so not in a place where you are likely to catch it.

And btw, if the composer file is done properly, this exception should never be thrown as composer would fail to install the bundle when curl is not loaded :)
wasn't that 0x80000000 ?
I think Erik purged the CRLFs that were already there. In my copy of this branch over to rightscale_cookbooks there are no CRLF issues.
I think you're right. I was currently figuring out that the values of the gradients had changed, since changing beta solved the problems (the algorithm is actually quite sensitive to the value of beta -- therefore it's important to work with normalized images!).
Actually, now that I look at this again, I don't have a problem with the above description. My main objection was the use of "transform" instead of "swirl" in the example.
No extra space please.
We should make this configurable, and at least as long as a long unicast-style broadcast..
WTF ? You should never catch the notices and even less assert that a Notice occurred as the code should **never** throw a notice.
Since it sounds like you're using sublime: please set the following options in your text editor:
    "trim_trailing_white_space_on_save": true,
    "ensure_newline_at_eof_on_save": true,

Coding convention is that every file has one empty line at the end of it.
I'll add that it seems wrong to have a list of specific formats in our code.  I looked at the code, but I couldn't tell why this was needed or what it accomplished.
I was trying to figure out if there was a way to better optimize this function and this sounds like the perfect solution.  My only concern would be if the extra branch is better than just barreling through.  Didn't we find a case before before where code was better without the if?
Mobile Web does not support this property
This should be formatted into multiple lines
Unnessecary var_dump here
Не стоит, эти штуки обязательны для нормального функционирования инпута, их нельзя оверрайдить.
shall we use "are" here?
And I tried to create the best possible one.
all of the removed code will be executed in ExpressionUtils.convertTypedValue(), so there's no need to do it separately here
Should this be a relative URL to handle deployments to other than localhost and on other ports
This pile of code allocates three sets of fifty strings each. That's a pile of memory that might nudge us into OOM on some devices.

I suggest killing `insertedGuids` and being more aggressive about designing for a constrained-memory device. We can't recover from this situation, so why not use two counters?
what is the reasoning behind the names gritter and chosen for css?
newer version of npm has a config prefix option that can be passed in, using that now, no more chdir!
Is there a huge difference here? I am guessing in 3.5 it will get every directory, but in 4.0 it will only yield them, allowing for lazy evaluation later down the stack.
whats this ?
As far as I know, demote-non-dropping-particle is only allowed on the cs:style node.
Што. С чего ты взял?
We probably want to make a jira ticket to switch this to `${firstname}`
If the duration is 0, we don't need to use animate here, and can just set it via css.
You don't need this. The commented out ones are mostly for stuff that is just using the Lua_V1 version.
Yes of course... silly typo. Fixed.
Do we still require the <code>config::file::set/get/remove</code> methods?
Shouldn't we always test allowDynamicStoredProcedureNames here?

Otherwise a message with a left-over header may cause problems when it hits another endpoint that has a fixed Stored Procedure.
The factory wasn't too hard to fix, but this is pervasive - there is a lot of code (within factories themselves) that expect TcpConneciton to be Runnable.

I propose leaving this for now and creating a new JIRA to refactor.

WDYT?
You're right, it doesn't make sense. The integral doesn't know its index, only the form does. I've merged already.
@Freeaqingme @Maks3w is right -- the verbiage in the old documentation is no longer correct for the new implementation, which conforms better with the mail RFCs.
I think having the builder used as a mixin is the wrong model.  Mixins define an 'is-a' relationship, whereas the builder is a self-contained entity.  I'd argue that it would be better to have a builder instance passed into whatever objects need it (which should only be the GitUpdater).
why duplicating the logic already done by the form component ?
codestyle: there is no empty line between @ param annotations. only between the different annotation types like return, throws.

also please have the type always first and the parameter name second.
Removed this piece of code and some more .
This is actually mixed, as you could use anything as the ID. I believe ODM will support integers if you use a numeric generator but the default is the string representation of a Mongo ObjectId.
Without 'de' this is a list of one. Is that okay? Also, I'm not entirely clear how this relates to my suggestion but perhaps I don't have the full context here...
It does check that the actor has the desired OrgRole in `actor_role_in_actee_org` which was intended to be the check that one would use in a role (and is used). 

However, looking at this again I think perhaps that the initial orgrole check should be moved to the decorator instead of being done at the top of the `actor_role_in_actee_org` method. I think we need to maintain the ability to call individual methods and if that was the case you're right that it is never checked here.
I think this should be implemented using other mechanism instead of inventing a new meta name
it should be private
Should gatherAttributes be called in the constructor(s) rather than on demand.  It would mean these functions could become const, and remove the need for the gatheredAttributes variable.  

(I started wondering if these functions need to be thread safe - possibly - and that would ensure they are.)
In this case, it would be best to use Doctrine\ORM\Query\TreeWalker, you can add where statement easilly, its not that complicated as translatable. Using treewalker allows to chain them, etc query can have many treewalkers nested, but it can have only one sqlwalker.
Please don't put this style of comment here. Module-level docstrings should provide helpful information. Committer information is handled by Git.
Why was this changed to `vzIndex`? This is not called out in the spec and no documentation was changed.
Might be nice to push all of these version checks into CDVLocalStorage
You learn from the best.  :cat:
Will do in next pull request.
You should check that $repository is a valid object I think before you use it as one.
I'll look again but I am pretty sure the super call is what caused the problem
you should add the resources for the ConfigCache, so that it can track changes in the translation files in debug mode. With your current code, the only way to refresh the JS file is to clear your cache
I'd rather not include this, then. If you're just looking for rapid prototyping, then this should work:

```js
 new Backbone.Marionette.CollectionView({
  collection: new Backbone.Collection()
});
```

Including this code in the core of collection view would give the impression that you don't need to pass in a collection yourself, which is going to lead to some really bad design problems and other bugs.
Nevermind, changing it.
Am I missing something or doesn't this only break BC if someone is using `render_model` directly?
I suggest just adding the geolocation code to `map.js` rather than creating a separate file - it's not really worth loading  separate file for such a tiny piece of code and all the assets are very cacheable.