labofsahil/github-event-dataset-2012 huggingface dataset:
I moved it, just didn't make the pull request yet...

On Thu, Oct 18, 2012 at 11:06 PM, Aurélien Thieriot <
notifications@github.com> wrote:

> In src/yammer.coffee:
>
> > - reply: (user,yamText) ->
> > -   @resolving_user_id user, (user_id) =>
> > -      if user_id
> > -         params =
> > -            body          : yamText
> > -            direct_to_id  : user_id
> > -
> > -         console.log "reply message to #{user} with text #{params.body}"
> > -         @create_message params
> > + send: (user, yamText) ->
> > +   if user && user.thread_id
> > +     @reply user, yamText
> > +   else
> > +     og_url = null
> > +     og_fetch = false
> > +     urls = yamText.match /\bhttps?:\/\/[^ ]+/
>
> Fine. Go for it then.
> You may want to move the OpenGraph thing in create_message but that's
> another PR
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/athieriot/hubot-yammer/pull/10/files#r1883584>.
>
>



-- 
/Ran
http://tavory.com
The method name is not following the naming convention for RH.
KeyValueStore does not support mapped superclass. So any mapped class shuld be considered as an entity. This method should simply return ``true`` in all cases.
Fix the import order: "import"s before "from" imports, and stdlib on top, third-party apps afterwards. So in this case:

    from os...
    from zlib...
    
    import gearman
    from django....
We could even use our `set->predicate` here :).
Small typo: picker -> picked.
You could also make it in two lines (but that's neatpicking).
The DI validation will complain about the parameter not existing, but doesn't actually tell a developer how to resolve the issue.
Is this URI directly to the comment or URI to all comments for the post?
Never never never define class without inheriting from `object`. This is difference between new-style (what we are using) and old-style Python classes.

I would rename this to `ErrorMessagesMixin`. Mixin is a class which does not really have a purpose on its own.
Why would there be one default connection? What happens in your scenario if user wants to leave default connection but keep others?

BTW, we are not doing IRC clone. All connections are equal.
Describe a bit more what is needed to be done here. `:` after TODO.
Don't have empty lines in blocktrans. Just add this section until the end. And then token on its own.
I think ticket is saying that user images should be 20x20 px? Is that too small? I don't think so.
Store also total. It does not matter that data is not normalized. It is easier to search like that.
I'm not saying '' is worst than some text. I was saying that its hard to debug things with no debugger except print :D
You mean like I had it before? Like 
                    
'image_url': '../images/logo.png'
shouldn't this be a scalar prototype instead ? I don't see the reason to add an extra level
`<p>`?
:). yeah, I included it just for comparison.
у тебя потоки, поэтому list(self.clients.keys())
клиент пожет подключиться-удалиться когда ты итерируешься по ключам
This doesn't look right to me. I'd expect to see some instructions testing the current cell for 0 and if it's 0 jumping to the instruction following the matching '] instruction.
elevHeatmap is a global so that it's accessible on subsequent calls to the showElevations method (so the heatmap can be removed from the map before a new one is created and added).  I don't think it could persist as an attribute of the Heater object since a new one is created each time showElevations is called.  And I don't know how else to make it accessible on subsequent calls - is there a way?
This should not have been changed.
Imho this old-traverser does not the correct thing as it would execution the traversal over the wire for each step, it should rather use RestTraversal.description() to build up the query (in a similar fashion), which would be executed on the server-side.
This should be f(1000,1225) -- fixed point constructor needs integer arguments.
I would suggest using the canonical_url (from plone_context_state) here instead / in addition.
I have it as both.  I thought it was cleaner to have it as an OLS 
results and then call this function in a separate module.  The  reason 
is that if I were to move everything over to the OLS results, I would 
have to move some of the optimizing functions as well.  If there are 
more additions to OLS results (bootstrap etc.) it can get very messy.  
That way, I thought it would be cleaner just to make a call from OLS 
results to this module.  I suppose I can still do that but not have it 
as a class, just individual functions.

On 08/14/2012 12:55 PM, Skipper Seabold wrote:
>
> In statsmodels/emplike/elregress.py:
>
> > +            parameter of interest
> > +
> > +        Returns
> > +        ------
> > +
> > +        llr: float
> > +            log likelihood ratio
> > +        """
> > +        return self.test_beta_origin([beta], [self.param_nums],
> > +                             method=self.method,
> > +                             start_int_params=self.start_eta, only_h0=1,
> > +                             stochastic_exog=self._stochastic_exog) - \
> > +                             self.llr - self.r0
> > +
> > +
> > +class ElLinReg(_ElRegOpts):
>
> Now that I'm looking at this in a bit more detail. As I think Josef 
> mentioned why is there is separate class for this? If this isn't an EL 
> estimator then it should really just be an option to OLS as you've 
> done elsewhere.
>
> —
> Reply to this email directly or view it on GitHub 
> <https://github.com/statsmodels/statsmodels/pull/406/files#r1374311>.
>
see http://api.nhindirect.org/java/site/gateway/2.0/apidocs/index.html

this can pull from a local file
this should just be 
```
public static boolean isNotNullish(collection value) {
   return value != null && !value.isEmpty();
}
```
Dude needs to work on his/her line spacing. There weren't enough newlines in the previous parts of the code... there's too many here.
please run jslint on your code before submitting a pull request
Pushed, added to 5.4 with other FITS I/O functions.
No, not intentional.  Just a product of copying the template and forgetting to change the comment.  I'll add comments like what you suggest.
> Actually, it used to be there. It just used to be done automatically.
> 
Yeah, I figured that out when I read onwards into the .h files.  (This is the danger of commenting on the python when it comes earlier in the diff!)
> It took me a while to figure out why 360 degrees / 20 = 0 degrees.
> 
Ew.  That is evil.  So you were trying to make the division of the ring for the ring test and the orientation wasn't changing, that's bad.
These should probably be in their own .cpp file.  They do have their own .h file, so we should keep that structure parallel.
Typo, have pushed a fix
Maybe. The LWRP uses a REST call to see if the server is already licensed.

For the issue you are bringing up here, maybe I want to do something like:

`NexusCli::Factory.create(nexus_cli_credentials).kind_of? NexusCli::ProRemote`

If the NexusCli::Factory class creates an instance of ProRemote, we know we are working with Nexus Pro and Nexus Pro *needs* a license to do anything useful.
I have removed them. I thought I saw something strange in the logging with the ports.
em-dash looks weird, I used `\`` - is that what you meant with "tick" ?
Je couperais la phrase en 2.
Again, "clears". `Clears an object's own properties` should be sufficient.
requested
`Settings`와 `Talk`는 안드로이드 아이스크림샌드위치에 `설정`과 `토크`로 되어 있기 때문에 그렇게 번역하는게 좋을 것 같습니다.
run-tests trim()s the sections. Doesn't matter how many trailing new lines there are :]
yes because a protocol might want to update its internal settings differently if needed.
long method calls can be wrapped
Can this be simplified? It's not clear why we have to manually build out the return value.

``is_array($all) ? return $all : return array();``
No wonder. Brah, the namespace is `git_reference`. ;d
added pagination class for pagerfanta
Can this comment be deleted?
Can these options be split onto multiple lines too?
based on above comment this should just be apiConfig.username
Shouldn't this be replaced with assignment to super.visible setter rather than just remove the line?
Потому что к private полям имеет доступ только методы класса (ну если без reflection)
throws RuntimeException писать необяазетельно. Пробел перед {
А разве не будет плохо если эту функцию вызовет несколько потоков одновременно?
Здесь табы
3 проверки !s.isEmpty() - надо бы упростить код.
Подчёркивания не допускаются.
Надо тест написать на случай inner class.
Сделай, пожалуйста, более понятное сообщение.
Why not put the optional `"`s into the regex and just don't capture them? Seems simpler.
I think you should stick to the locator you made in pull 58:
#file-list > div.preview
Hopefully, all denoising filters would reduce noise.
Hm, you'right, I might get rid of one of them.
Hey, the output doesn't make much sense to me but I would suspect that the problem arises here.
I going to look into it asap, but earliest late tonight. sorry..
It does have a slightly larger computational cost than simply finding the mode, but as soon as we allow other priors than the default one, the overhead is inevitable: to predict, we must compute the posterior for each class. (See my answer to your other comment below.)
We need all entries in the diagonal to `value`
This docstring is not entirely clear to me. If you read it naively, you'd just reweight all the predictions and it wouldn't have any effect.
"after each iteration of boosting"?
I think "InitHandler" would be better renamed "StartupHook" here.
this is an assumption about the method name used to set the locale. the clean way would be to fetch the metadata for $new and then set the fieldvalue of what metadata tells is the locale. (i think something like

    $meta = $objectmanaerg->getMetadataFor($new)
    $meta->setFieldValue($new, $meta::locale);

(not sure about the exact names out of my head)
Yes. Actually the processing of 'analyticstool' here should be removed since they are redundant.
I'll fix this once I figure out what I'm doing with everything else.
ctor is declared as private, more over it doesn't have parameter. so it can't be used for type converting.
Anyway made it explicit :)
You should still keep track of the type in a member as the type can be polled using getType.

This can be kept for the part2
These need to be actually defined as Javascript constants, so users of the Ti API can use them.  Also update apidoc
Thanks for checking it out
copyrights..
> I'm not sure this is a good long term way of suppressing the warnings.

Different people have different approaches. ;) I've seen at least three.

I'll change to commenting the parameter.
>  shouldn't it check that 'logicalname' != file->queryLogicalName() ?

Possibly. Preferably checked in doRename, not here.
This doesn't look right to me (and only the writer is supposed to modify that field, according to the comments)
5 is the charm... ;)
Can this use """ instead? Hard to read such a long comment with #
Why does this throw URISyntaxException? That's weird.
(fwiw, this should be eqivalent to `return nil`)
Change to the DB to ensure that it is enforced not NULL
Use parens around the arguments, e.g. `def sh(command)`
Also, shouldn't we have a check to see if $lang is different than $session->lang? If they're not different, there's no reason to do the rest of the work below, as it's already been done.
As noted above, the above (and below) @uses annotations can be removed.
"$reader" again... or $configReader?
it would be captured in kwargs, just like it was in save_email_message
What if there's no endpoint? Error checking/handling? Or do we just let it fail when you try to connect?
Need to place it in addWorld()
yeah, I saw it is an optional dependency after commenting
space before (

also, this is overly simplistic for i18n purposes (especially for some horrible asian corner cases) but I won't make you handle those :)
Are we interfacing with PayPal here? If not, this should get a real variable name.
The resp argument is a return value from send_request_cgi() in winrm_poke().  That return value can be nil sometimes due to a timeout, and generally any function using that should be checking nil as well.  You should do something like:

	return [] unless resp and resp.code == 401

Other common mistakes or bad practices you should avoid can be found here:
https://github.com/rapid7/metasploit-framework/wiki/Common-Metasploit-Module-Bad-Coding-Practice
The reason I did this was because the newline was extraneous and actually breaking moxie's app. You might could make the argument his code could be a bit more defensive, but I don't think this is required and it can only cause more problems later. egypt also ran into this issue recently. His main concern was that this could affect modules relying on the newline (if they exist to begin with).
Could be. But IMO a bit more of template code should be moved into partials, so maybe new issue about such refactoring ? (for this PR I will move the code)
Could you add some comments to indicate how this differs from the defaults?
Lets move `show_pagination = False` and `num_pages = 0` right in front of this.
I followed the code sample on the pytest [site](http://pytest.org/latest/example/parametrize.html) but I'm open to other options. Tell me what you think. In practice they both seem to have similar readability.
The second one probably can, although it will still need post-processing once loaded.  The first one is a file that won't exist if the user didn't build the documentation, so Sandcastle can't fail just because that file is missing.
I'm not sure about calling `clone` here.  Perhaps we don't have to since the DDR will play nice.  Famous last words.
Should the tiling scheme be user-specified?
For instance... if it is in the same project. Otherwise, same class.
Please note that SourceDirectory was never used by our scaffolding prior to this change. According to the comments in ScaffoldFile.cs, SourceDirectory is supposed to take a value that corresponds to a key in the parameters collection. The original implementation didn't do that, but since it was not used anywhere, the broken behavior was not visible. My change implements the "correct" behavior according to ScaffoldFile.cs.
Should 'android' be capitalized?
That s why i can sizeForFont with ```initFrame.size.width``` which is computed by taking textPadding into consideration
```
CGRect	initFrame = CGRectMake(initialLabelFrame.origin.x + textPadding.origin.x
									, initialLabelFrame.origin.y + textPadding.origin.y
									, initialLabelFrame.size.width - textPadding.origin.x - textPadding.size.width
									, initialLabelFrame.size.height - textPadding.origin.y - textPadding.size.height); 
```
I do consider this bad code. It's wasteful and over-engineered. If the other modules can use the style of code I suggested, then they should.
Can we proof the grammer and spelling here (ex: don't use b/c). Also if we can trim the length down, but
breaking some onto their own line.
We may want to do a null check here.  I know this doesn't happen in processInstantMsg(), but if we want to use this method again in the future it would be good if we handled a null serverName here.
I'll have a go, but last time I did that I made a right mess of things.
Why did you do that ?
Unfortunately no -- i've been using the multi-regions in my project for two weeks now. :(
Can you make this title case?  pypi.python.org allows lowercase to work, but this won't work with any other indexes that don't support case-insensitive search.
$this->path is never defined.
you use this same `font-family` [above](#L16R25) so I'd throw it in a variable. LESS is hot like that.
I thought, as I was referring to 1 codepoint it can be  'digit' not digits.
But I'm no expert in english :)
My gut tells me it's fairly subjective.
Given that D compiler won't let me fallthrough I don't think it's a concern at all.
From the aesthetics point of view I don't see a need for break after the last default case in a switch statement.
I've seen such functions for multidimensional arrays in other libs. Never found a good use for them.
On 26 mars 2012, at 11:42, Hardy Ferentschik wrote:

>> @@ -69,7 +69,7 @@ public ValidatorFactoryImpl(ConfigurationState configurationState) {
>> 		// HV-302; don't load XmlMappingParser if not necessary
>> 		if ( !configurationState.getMappingStreams().isEmpty() ) {
>> 			metaDataProviders.add(
>> -					new XmlConfigurationMetaDataProvider(
>> +					new MetaDataProviderXmlSource(
> 
> Or just :
> 
> * XmlMetaDataProvider
> * AnnotationMetaDataProvider
> * ProgrammaticMetaDataProvider
> 
> This does also not conflict with _ConfigurationSource_

FYI Metadata is a first-class word http://en.wikipedia.org/wiki/Metadata, no need to camel case on data.
`.debug`.
You can avoid some work here:

```
if (commandsProcessedShouldUpload) {
  return true;
}
long now = System.currentTimeMillis();
…
return age >= CLIENTS_TTL_REFRESH;
```
I'm curious: what can you do when you catch a MissingParentException. I neither see a unit test which mimics the behavior and explains it. Please add one, explain it in the jdoc, ...
Correcting myself, there is startupInHeadless method, so why this one?
Could we add a bit of formatting by tabbing this in since it is a sub element? Just for clarity later on.
The class name should match the file name.  Can both class and file name reference MCollective, since this is MCollective specific (there could be a SSH-based resolver or a sensu-based resolver, etc).
Since jsonp allows non trusted sites to send requests on behalf of users and see the response, it should be noted that implicit authentication mechanisms must be disabled if jsonp is enabled (e.g. cookie or authorization header based auth).

The bundle could even disable these itself (or e.g. allow to configure a whitelist of auth providers, or remove cookies and authorization headers from the request) during jsonp requests.
yeah its a bit brute force .. but easy to use .. think for most users that will be fine.
This comment is valid from C# point of view, and here it represents just a note that I put for a future reader of this text. I don't thik we should go that far to declare strict rules on such minor things.
Its in the stdlib.
Wait... I'm confused now... `download` refers to the DTA download, while `d` refers to the metalink descriptor.
Shouldn't it be `download.totalSize != d.size` (`download.totalSize && download.totalSize != d.size)`considering the previous comment)
Okay. I think I was confused because your comment first addresses utility, and then weight, but you define the arrays in the opposite order, so I was thinking of them backwards.

student_utility_weights is based off of enrolled classes, right? If so, you should mention this in your comments, because I first assumed that it based on all classes you asked for in the lottery.
what does the .dcm.xml extension mean? is it a common core of phpcr and orm?
Наши инклюды идут после стандартных (а совсем стандартные — до кутишных, это про stdexcept).
For the call signature, I would like to avoid using explicit defaults for things like linewidth and color.  Instead, I would rather that the default be set to None and if they are None, then grab the rcparam values.  Don't know if there are defaults for arrowsize and arrowstyle, but maybe one should added?

Even better would be if the color (when None) is pulled from the existing color cycling mechanism! But I would be happy with just pulling the color from rcparams.
Part of a 3rd party plugin.
@joliss fixed in #ce76059
Why are you removing the comments for the enum values?
Is documentation generated from the `.di` file?
(Fix Submitted) - further more, to ensure this does not happen again a sleep for 10ms has been added to the tests.
The Mesage is back in the DB and with poler being set for 200 fixed rate it
was polling for the same Message over and over again.
As I said to Mark. I am a bit surprised that this issue didn't come up
before.
In fact a simple Thread.sleep(anything. . . e.g., 500) will reproduce this
error consistently.
I don't see aby reason to introduce this method.
Also I like Chris's comments for contributors about tests: https://github.com/SpringSource/spring-framework/wiki/Contributor-guidelines
Paragraph: **Submit JUnit test cases for all behavior changes**
When we should mention JIRA for unit-test.
No, it no longer relies on 'delete not completing'. In fact i changed this method back to 'void' (as it original was).
But I do agree that this could be simplified into something like this:
```
if (!this.replyDestinationExplicitlySet && 
	this.isTemporaryDestination(replyTo) && 
	!this.isCachedSession(session)) {
				
    this.deleteDestinationIfTemporary(replyTo);
    this.tempQueuePerSessionMap.remove(sessionId);
}
```
Why you've changed the way to instantiate Model objects? The SDKs Model use the form of having default constructor and you set the values via properties.
This is quite annoying :/. In my Markdown editor adding one make it longer :/.
Shouldn't this be a third-level header:
```
Basic Usage
^^^^^^^^^^^^
```
(This would apply to other headers you do later that are marked as simply bold...)
@apex13 This should now be fine as I merged in `master` which should contain the regular_list CSS class. I also changed the indentation of the `<li>`'s
The name of this class should be related to what it actually does ... perhaps ARVPatientSnapshot
I prefer to use non alias name for query.

And you can group your select.

```php
<?php
$qb = $this->createQueryBuilder('student');
$qb
    ->select('student, professor, tutor, contract_type')
    ->leftJoin('student.professor', 'professor')
    ->leftJoin('student.tutor', 'tutor')
    ->leftJoin('student.contractType', 'contract_type');

return $queryBuilder->getQuery()->getResult();
```
should this be in the rc file?
*all* -> *allow*???
Tests inside `*_support` modules should not rely on the behavior in any of Armstrong's `TestCase` classes.  These should be straight Django.
Good point, will change this assertion
hamchin readme age nabashe behtare
копипаст детекстед
How come no more querySpec property?  Just for cleanup purposes?
Make clear, that the tool environment is also a model
Do you mean `$ puppet node_rackspace reboot 12345678` here?
``mail`` can be confusing for new comer, maybe use ``default``, or ``catchall``, with an empty routing key.
Why have you deleted this classes?
Minor: Or code conventions typically favor this style:

    try {
        ...
    }
    catch (Throwable t) {
        ...
    }
А чому только форчан? Що мешает ея использовать на иных бордах?
We still do it here - https://github.com/blackberry-webworks/BB10-WebWorks-Framework/blob/next/lib/framework.js#L65
No opening brackets on new lines
This define requires a comment.