labofsahil/github-event-dataset-2012 huggingface dataset:
I don't feel strongly -- it would be easy enough to do:

```
typedef enum { DICTIONARY_FOUND, DICTIONARY_ADD, DICTIONARY_FULL } dictionary_lookup_result;

        switch (__rec_dictionary_lookup(session, kv, &dp)) {
        }
       
dictionary_lookup_result
__rec_dictionary_lookup(WT_SESSION_IMPL *session, WT_KV *val, WT_DICTIONARY **result)
{
        for {
               *result = dp;
                return (DICTIONARY_FOUND)
        }
        if (empty != NULL) {
                *result = dp;
                return (DICTIONARY_ADD);
        }
        *result = NULL;
        return (DICTIONARY_FULL);
}
```

if you think it's worth making that change, I'm happy to do it.

Or did you have something else in mind?
Maybe add a comment since this CATCH block is empty?
- Are you sure ``$id`` has the correct type as ``integer|string`` is not a sub-type of ``string|array``?
- Are you sure ``$eventName`` has the correct type as ``integer|string`` is not a sub-type of ``string``?
Should the type for parameter ``$container`` not be "null|Container" instead of "AbstractContainer"?
Are you sure you can call ``mt_rand()`` with the given parameters? It seems like at least the 1. parameter (``$min``) is missing, no?
should be

```ruby
job = PerformLater::Job.new(queue, klass, send(:class).name, send(:id), "now_#{method_name}", *args)
job.enqueue :delay => delay
```
needed?  doesn't seem to be used
Well, couldn't it just use the local 'id' variable instead?
Also, it's 0, 0, Plugin.

By the way, if we're going to add the separated client/server plugins, we should also have a system that stores if a specific file has already been loaded and the Plugin instance it was assigned, so re-executed plugins can use the same scope as last time they were executed (for variable persistence, etc).
Webapp, Schwebapp...
Stimmt...
I agree with @philfreo - or they should be moved up to the end of the events section, it's one of the most important things to understand about backbone and doesn't make sense to have below the giant list of "examples"
I think this would be better as 3 separate tests.
No points from this one.
More since tag changes?
Can you really typehint a resource ?
`nil =~` returns nil (at least on 1.9.3) so I don't think you need the length check here.
Changes made to the sprunkrc docs in the README should also be propagated here (among possibly other places).
Fixed on e58c449e2ab2e877855d885c58da6da3126e8cc1
Space before `()` in anonymous functions.
GROAN. Import only modules.
No, fieldset is something else. Not that every row is a fieldset.
And you dare to publicly tell me you are copy-pasting?
`oldc` is what? Be verbose.
I got this from wikipedia: "The field name can be composed of printable ASCII characters, 0x20 (space) through 0x7D ('}'), with 0x3D ('=') and 0x7E ('~') excluded."
I am uncertain about this point. Perhaps it'd be better to impose no restrictions on the PropertyMap level, and check the keys in the vorbis implementation. Appearantly APE has different restrictions (2-16 ASCII chars, not "ID3", "TAG", "OggS", "MP+"), so there's no real reason for using the vorbis convention (except that I have mostly Ogg/FLAC stuff).
I don't have a strong opinion in either direction ... I guess none of those restrictions will ever be relevant in practice. ;-)
I wouldn't bother with this one.
no need with boss ai
The three things that come up for me:

1) When registering an UncaughtExceptionHandler, it's generally a good idea to call through to the existing (or default) uncaught exception handler.

2) Given that this is code which exists outside the lifecycle of a service, it seems a little odd to have it exist as an inner class that maintains a reference to the service.

3) You're going out of your way to call a static method on an instance, which doesn't generally lend itself to readability for what's happening.
Recycle. nice.
If the feature extractor happen to meet an unseen word, it will give this
unseen word a default value, instead of giving nothing. It's an idea of
smoothing. Here we set the default value to me the mean. Please let me know
your opinion.

Best,
Chen

On Thu, Aug 30, 2012 at 8:43 AM, bethard <notifications@github.com> wrote:

> In src/main/java/org/apache/ctakes/feature/extractor/WordValExtractor.java:
>
> > +import org.apache.uima.jcas.tcas.Annotation;
> > +import org.cleartk.classifier.Feature;
> > +import org.cleartk.classifier.feature.extractor.CleartkExtractorException;
> > +import org.cleartk.classifier.feature.extractor.simple.
> SimpleFeatureExtractor;
> > +
> > +
> > +
> > +public class WordValExtractor implements SimpleFeatureExtractor {
> > +	
> > +	private Hashtable<String, Float> word_val;
> > +	private Float default_val;
> > +
> > +	public WordValExtractor(Hashtable<String, Float> wordVal) {
> > +		super();
> > +		this.word_val = wordVal;
> > +		this.default_val = calDefault();
>
> What's the purpose of "default_val"? It seems like this feature extractor
> should not return any feature if the word can't be mapped to a float, right?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/bethard/ctakes-temporal/pull/1/files#r1496126>.
>
>
0b19557
This is correct, but could you define a property instead in the root pom? Like the EHCache and Infinispan versions do as well.
We need that to make sure documentation versions are consistent, and possibly if it's needed in other modules.
Why is this _synchronized_ ?
About the naming, I'm not sure that stream_config is explicit about what this does.
Maybe should we put this in` circus/stream/__init__.py` rather than in util?
Good point about the `.filename` and co. The `.short_distinguisher` handles the various cases nicely.

As for the ellipsis, again I tend to think that less markup and CSS is better esp. when we are talking about one character.
`-= 64`, maybe a const to annotate it?
Quick question, would it be possible to restrict the search there instead of looping over the results afterward ?
This is not necessary anymore
You should be able to get plural rules using `TwitterCldr::Formatters::Plurals::Rules` class.
You're right, the if branch is not consistent with our scheme of having these base classes so that users don't have to interact with SBProfiles.  Will change to use only the else branch (and also get rid of the *args version).
I think SBAiry has getD(), not getLamOverD(), so this won't work.  However, maybe this would be a good opportunity to switch the SBAiry implementation from taking (and returning) D to using lam_over_D like your optics stuff does.  Especially since D is really a misnomer, being in fact D/lambda. 

Also, this somewhat indicts the unit tests for Airy, since we apparently don't have a unit test that noticed this for you.
I think it's probably still something I'll consider a long-term TODO item.  But I agree that we can remove the explicit mentions of it as a TODO here (and elsewhere), since the workarounds are very reasonable.  For example, a user can read in a config file in their own python code, do whatever calculation is necessary, add those fields to the config dict explicitly, and then run `galsim.config.Process(config)` to do everything specified.  Or they can even construct their own config dict within their code, complete with any python calculations they want to do.

The thing that might be convenient to do that isn't currently possible is to have some generated value be a function of other values that have been generated for that object.   The most prominent example is `gal.resolution`, where the galaxy size is a function of the (possibly random variate) psf size.  I added that functionality explicitly, so that's taken care of, but I can imagine wanting other things than that.  

So I have (not fully formed) in my head an idea of an `Eval` type for float_value and maybe others, where the user would provide a string to be evaluated.  The tricky bit is to have some way to access other generated values (like the psf half-light radius) to make it actually useful.  But I think it is appropriate to wait for user demand before trying to implement it.
Whichever you want.  I usually wait until people finish a round of commenting so github doesn't mess up the locations of comments.  (I think that's what is going on when comments get mysteriously moved to other locations.)  But if you want to edit anything directly, I don't mind.
Same here. axisCount should be used for readability and to save memory.
Why are you dividing by 100 then?
Expect there's a nicer name than newEntity2, but that's not from this pull-request...
If Success(String) why ExtendedResult[Any]?
Sorry - I don't understand - this gem still works exactly as before, unless you pass in a string that starts with '$', '£', or '€'.  The current version of the gem ignores these and assumes you want USD.
OK, 
Effectively I also prefer the Ext 4 way, I just say that not a big issue to migrate to Ext JS 4 :-)
CU
Please remove this from commit. Add this folder (or file) to .gitignore file in your project
Remove this line break
I just ran the tests right now after adding in ``@cython.cdivision(True)`` for all four pyx files and got no segfaults (although one of the tests did fail). It's only a ~5-10% speedup to add this (for a 500x500 array w/ 3x3 kernel), though, so perhaps it's not a big deal.
fixed in #3285
That isn't duplicated the second pass is using values previously set on the first pass. It is used while on digital zoom.
updated. Sidenote: since 0.6.0 the tracebacks actually become useful. Cant wait to go there...
args[i] надо в отдельную переменную вынесит.
Какая странная ситуация. Когда она происходит?
I didn't realize .toString was overridden.
I'm not quite sure why this class exists, is it just for organizational purposes?
hmmm...  don't know what happen there. Added the space :D
s/Check is/Check if
I wasn't sure if we should have a separate TextWallPost or include a text field in the WallPost class with the idea that most posts will have text. I went for the separate TextWallPost because I know from the designs that system wall posts don't have text. What do you think?
>    I raised the question before. I would say no but they if need to maintain
>    backward compat to store coef_path_ as attribute

OK. How much of a performance difference does it make?
looks like the name hasn't changed. you guys must have some affinity for triplets of the same letter ... fff ... sss ...
@amueller It is done.
I've assigned `fit` to the EM algorithm, so I think setting `filter_update` to `partial_fit` would be a bit confusing.  It's more like a `partial_predict` than anything, but there's no API for that, right?
Minor point -- code using yield cannot be serialized, but it is often helpful to serialize long-running code such as hyper-parameter optimization.

Consider making `__iter__` return self and adding a `next` method that returns a params sample?

I'm happy enough to do this as a PR of my own later if you like, so that any discussion can be separate from this PR. Anyway there may be other serialization problems, this might be necessary but insufficient.
this sentence is a bit hard to parse. Maybe replace the first "or" with a comma and say "otherwise" instead of "else"?
ok, cool, then it is even more flexible than I thought...
I guess O(n_samples × n_classes) can be a bit big indeed. But that was an interesting little snippet, thanks.
Коментар додай до методу
If you add ``warps(fn)`` it will add the original name of the function and docstring back to the decoratored function. It's not critical, but it's a nice thing to do for people poking at your code in a REPL.

http://docs.python.org/library/functools#functools.wraps
`arguments, 0`
No brackets!
I ended up using hashCode() of accountName passed to the notification method. Eventually it does the same and it's less code.
I dislike it too, but it's the man's prerogative. Don't change it, but no need to use it in new code.
Ah yes, I've gotten used to the D way of things. :)
The document says:
"If a color property is undefined, the default color of the particular UI element is applied. If a color value is not valid on iOS, the default color is applied, whereas, on Android, the color yellow is applied."
I've made a note of this in Jira task https://jira.appcelerator.org/browse/TIMOB-9457
use `= NULL`
Let's put this in a view instead :)
```php
<table class="table">
    <tr>
         <th>Id</th>
         <th>name</th>
    </tr>
<?php
foreach ($this->bugs as $bug) {
    ?>
    <tr>
         <td><?php echo $bug->getId(); ?></td>
         <td><?php echo $bug->getName(); ?></td>
    </tr>
    <?php
}
?>
</table>
```

I think this is enough for a view :)
One can create a package for compass that just calls gem install compass.
ModeShape needs the regex JARs, too. I guess you don't use them?
Is this a measurable improvement? The code is quite complex as is.
What is the commented-out bit waiting for?
These are purely derived attributes - they're not used for checking if equal.
ifconfig -a + grep and awk would be the only real way in bash directly.

But I do have an idea, Maybe add a function to return the local IP from configgen or a similar app. It would be best and using jlib it can easily be returned.
The minifyer will take care of that right ;)
Same here, use the django forms.
It's said in http://nova.openstack.org/runnova/vncconsole.html#important-nova-compute-options

The API call is passed to the compute node hosting the instance which then gets a token from nova-consoleauth an returns the url + token
cf. code here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1640-1665
Без этого точно не работает?
Feel free to join our #automation channel as listed on QMO: http://quality.mozilla.org/teams/automation/ I'm in the same timezone and should be around that time.
Entirely undocumented class. Not good!
Ah, but this does not seem to be related to this bug fix (unrelated changes must be avoided at this stage in the release cycle!). Please amend.
Nice touch! :+1:
Instead of hard-coding a value, this should be an optional key, like 

(defn start-job-flow [name steps {:keys [log-uri bootstrap-actions ami-version supported-products visible-to-all-users]
                                  :or {bootstrap-actions [] supported-products [] visible-to-all-users false}
...
(.setVisibleToAllUsers visible-to-all-users)

default should be false (compat with std EMR).  We can change the default to true for TCC in our local codebase (in lemur_base.clj)

Also change com.lemur.core (around line 568) add this to the map:

:visible-to-all-users (true? (:visible-to-all-users evaluating-opts))
Adding some more JSDocs & we should show the defaults here.
hint (you probably already know, but): you don't need to specify this-> unless you're doing something ambiguous, like:

class a { private: int foo; public: void setFoo(int foo) { foo = foo; /* oops! */ } };
No general url's in references pls, only references to the specific technique being used.
Is the val_version argument really needed? Can we delete it?
remove old debug lines?
You removed the post conditional elsewhere, probably should do the same here.
Even though `_createTextureAtlas` is private?
Yeah, this is some of the oldest code we have as far as CZML goes.  It was all written by @shunter way back when; of course at this point it's my fault for not doing a full review of it before opening the pull.  I'll go through and clean everything up.
Some of them were, yes.  I'm not sure about others since I didn't write them.  There were some tests using simpler inputs, but the problem with simple inputs is that they don't always execute all of the code or stress any edge cases.  I do agree that some of our tests with more complicated numbers could probably use more details in the comments.
Similar to my recommendation for string constants for Material IDs, should we have a `Uniform` object that has string constants for all of the standard Cesium uniform names?  So instead of having the string `'agi_defaultImage'` sprinkled throughout the code, we have `Uniforms.defaultImage` or `Uniforms.agi_defaultImage` if we want the names to match.  Thoughts @pjcozzi and @shunter ?
OK, the image links was actually the first thing I noted when creating a CZML file today.
`animate` should also be specified before the annotation is added to the map.
Capital 'i'  on imported =)
Values that this can take should be defined in the UI Module.
In this case, there will always be a view and proxy child if things
get set up correctly.  I'll add more checks here for safety.
Maybe use the traditional "var i" instead of "var m" here.
You don't need setContent.

Just pass popup.entriesNode as the second argument to .render() above.
like that name. created a PR - feedback welcome: https://github.com/symfony-cmf/Routing/pull/33
I think I need an example as I dont see this way being used in any other plugin

Thanks
nit: the `x` should be `\times` and wrapped in `<code>` tags. Also, the `=` should probably be wrapped in `<code>` tags too.

Coloring equations is a bit cumbersome right now. There are a couple ways to do it. One is the approach you're using:

    <code class="hint_blue"><var>TAX</var>\%</code> <code>\times</code> <code class="hint_green">$<var>PRICE</var></code> <code>=</code>

The other is to use `\color{}` within MathJax:

    <code>\color{<var>BLUE</var>}{<var>TAX</var>\%} \times \color{<var>GREEN</var>}{$<var>PRICE</var>} =</code>

They're both cumbersome, but just wanted you to see both--sometimes it's easier to use one approach over the other.
`s/,//`
SSSE3
this line is a bit strange with so many quotes, would this work?
BASE_DIR=`cd $(dirname $0) && pwd`
/so an/from an/
Matt, 

I appreciate your feedback.. However in this instance, the return was being used for business logic and not for indicating any success or failure of this code in any way 
----- Original Message -----

From: "Matthew Batchelder" <notifications@github.com> 
To: "plymouthstate/plymouth-webapp" <plymouth-webapp@noreply.github.com> 
Cc: "bscoleman-psu" <bscoleman@plymouth.edu> 
Sent: Wednesday, October 3, 2012 11:52:55 AM 
Subject: Re: [plymouth-webapp] changes for adding new BM Report App (#425) 


In legacy/hub/HUBSQL.class.php: > +    {
> +		  $sql = "SELECT * FROM `bm_rounds` rounds WHERE rounds.bmr_id=? ORDER BY time ASC";
> +      $rs = PSU::db('hub')->Execute($sql, array($data['id']));
> +			$rounds = $rs->GetRows();
> +			$data['rounds'] = $rounds;
> +      return $data;
> +    }
> +
> +	return $null;
> +	}
> +
> +/**
> + * add a Round Entry for Building Manager Report to the MySQL hub database
> + * 
> + */ 
> +	function addBMRRound($now, $data) 
As my name was invoked and this is an open source project, I figure I'd weigh in on the policy that I had used during code reviews and my perceptions of this topic: 
Re: Best Practice 
When manipulating data within a database, returning the manipulation's success (or failure) is a best practice as it allows the code to react to successes/failures of writing to the database. While a return may not be used in the current version of the code, it future proofs it for: 

    * Logging 
    * Error reporting 
    * Error messages to the user 
    * Other unforseen uses of said function/return value 


Returning During MIS' Past 
During my last 4 or so months as lead developer at PSU, I would strongly encourage return values to be used in any code MIS developers produced and urge their usage in functions that manipulated the database. Flexibility of code was important to MIS and future proofing meant less work later. 
I dinged @nrporter , Laurianne, @zbtirrell , and others on that practice when they didn't follow it (and they did it to me when I forgot). My hope while I was there that all functions/methods going forward would follow that practice. 
Whether that is the direction that MIS wants to continue to take is up to them, but that's the recent history of this topic and it's working internal practice. It is important to note that that practice was not on the MIS wiki when I left, though. 
— 
Reply to this email directly or view it on GitHub .
same as others
How should this be to account for wrap around?
Ignore the above comment I've suggested another generic way to do that for all code segments like this.
This could have been done like this:
def populate_network_defaults(hash, prop = nil)

Then you wouldn't necessarily have to call the nil cases out.  not sure I care, but it is more stylistic
It's on the current Splash.  It'll probably go down as more people register.
Пробел после запятой. И лучше же все выражение после запятой на следующую строчку перенести.
Suggested fix works fine.
Can combine this and the next two lines
Can you make the titles i18n?
Could you update this:

* change `page` to `pagePath`
* add `callback`
These reply timeouts cause a lot of confusion for users; we should make it clear what it means, like we did in the latest revision to the JPA chapter merged yesterday.

""
			Specifies the time the gateway will wait to send the result to the reply channel.
			Only applies when the reply channel itself might block the send (for example a bounded QueueChannel that is currently full).
			By default the Gateway will wait indefinitely.
""

Regarding param - I would rather not remove it; change it to, say  @ param replyTimeout The timeout in milliseconds.
Maybe `if (isPseudoTxMessageSource && isInTx) {`
?
This is what I figured out when I first looked at it.
I imagined something like this:
```
$identity_column = ( ! in_array( $this->identity_column, array( 'username', 'email', 'id', 'password', 'active', 'last_login' ) )
    ? ( ", " . $this->identity_column )
    : ""
);
$query = $this->db->select('username, email, id, password, active, last_login' . $identity_column);
```
`{start_with_decade: false}`? ;P  It might be a better idea to simply expose all three views as values -- maybe someone would want to start on the year view?  I'm thinking a switch block which looks for 0, 1, and 2 along with human-readable alternatives 'month', 'year', and 'decade', with a default to 0/'month'.  Not sure about a name for the option -- startView perhaps?  Open to suggestions for that.
This will execute every time js file is loaded. We call this elsewhere already.
Also, it should probably say "The interface used to route public traffic by the network service."
++ on the README update!
What's the benefit of adding generics here?
型が長いです。
This is too permissive, allowing unbalanced indentation. Please refactor in a way that forces matching indent/dedent markers.
This is not guaranteed to work. The error trapped here and the exception handled here is very generic `Exception`

If for example a connectivity error or some other error occurs, it will not have the two parts you are expecting.

If you are sure you want to store these messages, then handle `PyUPSException`
Nope. This is the thing that remains to be done.
@bp323 why can't we use `params[:action].eql?("statistics")%> `? Just like the ones above?
let me put it this way: can (array.array.length < (offset + (count*stride))) ever be false after you reset count?
I know this is probably my code, but can you modify the example success callback to look like this?

wizAssets.deleteFile("file://documents/settings/img/cards/card001.jpg", successCallback, failCallback);

It will be cleaner for a JavaScript programmer to understand. Sorry for the confusion. 
I would like to keep this as the best practice for future readme files. I will go back and check previous readme files, but lets do this from now on.
I let you add the other WEBOTS macro and I will merge this pull request.
数据库表名要叫“plagiarism_moss_diff”，和其它表保持一致。

另外db/install.xml怎么没更新？
Empty tuples instead of empty lists?
Je n'ai pas testé avec la PR #3 de JB qui embarquait prettyprint. Je tenterai de modifier.
**Warning:** Please trim any trailing whitespace
indentation should be 4 not 2
Please remove all logs,
Take care of this.
This would break if Spree was mounted at another location, other than root.
Same as above, PEP8 compliance
@DPontes hello!
Mentioned as iOS-only above.
We need to be very clear about the following: remember/forget are not reference counted - multiple 'remembers' are all undone by a single 'forget' (known bug)
Totally slipped my radar...yes we can implement it.
*NOTE:* These do not need to be added as part of this PR, they will be evaluated separately
Ah I see. This was last minute name change. We should name all in the same way in the future version. Thanks NARKOZ!
It is like it is said in the comment. My version feels cleaner because it saves some unnecessary & duplicate calls. It should do the same.
okay, i think that was the reason why  i used named queues in the initial PR. As i see it, we need the named queues in the configuration because otherwise you can't get the correct queue in the worker.
did you mean to open port 80 on this template?
Could we not load a qunit test instead and verify that all tests pass?
А встроенный сборщик мусора с ентим разве не справится?
Если оно действительно улучшает картину, то надо бы аще поискать в скрипте места, где используются временныя масивы.
Should put a quick comment on why you are changing the value of keepSource to false.
`Don't keep source files if there is an error` or something similar.
Good question. I would think MailCore handles missing information OK, but I didn't test.
use Json::FastWrtier().writer() instead to use it as a static object, less memory usage
Why was this file changed to *not* include tests?  `tests.utils` refers to `<app>.utils`.  That's why we have `test._utils` in nearly every Armstrong project -- to avoid the namespace collision.
Would be good to have the text broken into multiple lines, is that possible?