labofsahil/github-event-dataset-2012 huggingface dataset:
owja goede punten ik ga fixen

2012/1/3 Rik Steenkamp <
reply@reply.github.com
>

> waarschijnlijk wanneer de pencil de rand van het scherm kruist. ik zie
> dat daar een if(checkcollision) ontbreekt!
> (free de newsegs ook trouwens (en alloc enkel wanneer checkcollision=1))
>
> 2012/1/3 Marcus Klaas de Vries
> <reply@reply.github.com>:
> > het gebeurt als een pencil streep iets kruist -- ik ga op onderzoek uit
> >
> > ---
> > Reply to this email directly or view it on GitHub:
> >
> https://github.com/marcusklaas/Achtung--die-Kurve-/commit/37d62c668e159855f0eb507ed67ab123d053daa1#commitcomment-834141
>
> ---
> Reply to this email directly or view it on GitHub:
>
> https://github.com/marcusklaas/Achtung--die-Kurve-/commit/37d62c668e159855f0eb507ed67ab123d053daa1#commitcomment-834213
>
@WalterBright, is that entirely accurate?
Hungover before sobering up? Impressive, I must say...
Right if you rewrite it, the quick will be easy. Just check for stateIds, stateUuids first and then conceptIds, conceptUuids, etc. as in the getWorkflow method.
As we talked about, I would trigger a build via v.exec('node tools/r.js -o tools/build.js') and wait for that to finish, then do the file scanning in www-built.
doesn't matter if RackServlet or RackFilter you still need to add `((HttpServletResponse) response).addCookie(cookie)` here :)
Там не пол-скрипта, не бойся. Я просто сделаю уникальный идентификатор для каждого поста, с которым и будут работать все функции. По идентификатору можно будет определить всё, что для них нужно. Сами функции почти не изменятся.
shouldn't this be equal?
@ericf: I was thinking about this and it seems like this stuff could live in a module that listens to any whitelisted DOM "Pointer" events (whether it be mouse, mspointer, or touch*), and fires some type of synthetic event and normalizes event payloads across the board which event-touch kind of does right now. 

Modules such as event-flick and event-gestures can then rely on that, and simply code against one synthetic event, instead of listening to various events based on feature detection. What do you think?
`o` is undeclared?
The title and the copyright should both go into message bundles.
The `else` clauses here are redundant. You could simplify the block with something like:

`return value == null || value.equals(nullPlaceHolder) ? null : value;`

...or better yet, use this instead: http://commons.apache.org/lang/api-2.6/org/apache/commons/lang/ObjectUtils.html#defaultIfNull(java.lang.Object, java.lang.Object)
I'd like to talk about this outside of review if you're concerned about it. Feel free to ping me at your leisure.

My concern about the multi-file approach (and, indeed, why I chose to depart from it here) is it opens up the system to several distinct failure cases that would eventually require a much more complex system. They are:

1) If Tron doesn't handle some form of a hierarchical directory walk (with or without symlink handling), we would need to find a way to get around the inherent file-per-directory limitations (http://stackoverflow.com/a/466596) or add these as non-guarantees.

2) If Tron does handle a hierarchical directory walk, we would want to address several subproblems here -- notably, how Tron should handle symlink loops, how Tron should handle nonsense or junk files found along the directory walk, etc.

3) Most nettlesome: the arbitrary, multi-file approach opens up entire classes of security problems that are difficult to address. In even the strictest of cases, one could serve Tron a local denial of service by, for example, dropping a junk file, broken symlink, or other anomaly in the target directory. This would need to lead to some form of reconciliation strategy, by either ignoring anomalous files (and taking the risk that one of them was a valid configuration file) or otherwise try to reconcile this ourselves.

A better solution might be to use some form of a configurator to help Tron handle multiple files cleanly in a Linux environment. Something like PyStaticConfiguration would not be wrong, but it would require discussion, because it would represent a major architectural departure from what is currently available.

YAML provides the ability to define complex data structures by design, and using a single configuration location seemed best for the reasons above, provided one can guarantee idempotency for each configuration change. That passes the burden of multi-file configuration to tronfig, which would be then be the tool to experience any denial-shaped scenarios (instead of Tron).

tl;dr: I anticipated this concern, and I'd like to talk it out at the next opportunity. I think either strategy is workable, modulo the drawbacks raised by both of us above.
Oh, sorry, can't do that for 2.0 as SerialVersionUID isn't fixed in 2.9:

//TODO add @SerialVersionUID(1L) when SI-4804 is fixed
yes for anything else but documentation,
I wan't to show the type returned from serialize
I'm with Roland on this one. Enhances readability in my opinion.
why not `import LsKeys.lsync`?
That you can not use a BalancingDispatcher with *any* kind of Router. 
Now that yields a ConfigurationException. 
So that violates this whole section of the docs.
I can see that you have had a long discussion, and I admit that I have not read all of it, but my instinct tells me that this specialized LoggingAdapter doesn't belong in akka. If you want to log directly to j.u.l. then you should use that api directly. Exactly what does this `JavaLoggingAdapter` solve that the ordinary `LoggingAdapter` together with the above `JavaLoggingEventHandler` can't handle?
(Integer/parseInt "666") 
; => 666
I don't think it's overengenered. It's just a property that turn itself into an attribute...
No, I'll merge it upstream
This seems to be a common way to implement enumerations in python.  At lease according to stackoverflow.com.

But, seeing how you don't love it .. how about I just add as constants in the class instead?

Like:

BIND_ACTION = 'bind'
UNBIND_ACTION = 'unbind'
PENDING_STATUS = 'pending'
SUCCEED_STATUS = 'succeeded'
FAILED_STATUS = 'failed'
In other places in the docs, for a third level section we've underlined with ^. For example:

  Listing Schedules
  ^^^^^^^^^^^^^^^^
I think this error message is incorrect. Why does it specify "version 2" instead of a substitution?
what about other cases? match exception is ok?
I was considering the same thing but the changes required are certainly more work and they touch more files as Update can be overwritten by every class derived from CGUIMediaWindow. But I agree that I would also prefer to save this extra member variable if possible.
Isn't that is why the decrement is there in the first place?
Ie, Find timestamp, take previous packet.
I don't know what style you guys normally use, but wouldn't

std::string deviceName = passthrough ? "audiooutput.passthroughdevice" : "audiooutput.audiodevice";
m_outputDevice = g_guiSettings.GetString(deviceName);

be way more readable?
Yeah, I need to update the PR. In the meantime I have almost fixed multi-head setups.
This could perhaps just be a QObject and use qobject_cast instead.
Was this the one causing the problem?
I would prefer None.
You can use getUnderlyingResourceObject() from the FileResource and avoid creating a new file object this way
c'mon guys, I provide you with a patch that adds test cases for features you've not tested and now you're complaining on curly braces?! If this patch is agains your CS, then prettify it yourself before merge.
We didn't like this, gone now.
This code was so last year.
I think the "case `remoteDaemon`" section should be moved into the "case l @ (_: LocalRef | _: RepointableRef) if l.isLocal ⇒" section, since the remoteDaemon cannot be anything else than local. And it gives me the heebie jeebies to send messages to remoteDaemon if we are in untrusted mode, right?
This shouldn't be blocking. The metrics processing function should take a callback, which then emits to the backend.
I'd prefer `data.content = content||file.content;`
`self.class` is sufficient to include a description.
Just left an issue here, because I think quite conversely: a space looks horrid to me. #programmerdebates https://github.com/github/objective-c-conventions/issues/18
Sure, I can do that if you think it's a good idea.

In order to learn from this, I'd like to find out the reason. Is it a style issue?

I chose to use that format to make use of the setAlwaysBounceVertical function, which would otherwise be bypassed if I did _tableView.alwaysBounceVertical = true;.
Both the ETW and Perf Counters implementations got largely copied from the DTrace one. Granted I guess ETW and perfctrs is a little simpler.

Getting rid of the extraneous `b() == 'true'` seems like a clear win (committed, pushed), but the one-liner less so.
Use `args.This()->GetIndexedPropertiesExternalArrayDataLength()` here.
Let's eliminate the need for this kind of stateful logic.  If we really, truly need a public init method, let's make it hard to use incorrectly.  Here's my current thinking on the topic:

Add the following function to z_hookset:

int libd_safe_init() {  // returns true if and only if an actual init was performed
  static int initialized = 0;
  if (initialized) return 0;
  initialized = 1;
  libpd_init();
  return 1;
}

Let's call the public method just Init, not ReInit, and have it call libpd_safe_init instead of libpd_init.  Now it can be called at any time, and it'll only do something if the DLL has been newly loaded.

Also, let's call LibPD.Release() in Init, or else there might be leftover state on the C# side.

With this kind of safe public Init method, I'm no longer sure we want to implicitly initialize libpd when the class is loaded.  This might give rise to confusion since there are cases where you have to call LibPD.Init() as well as cases where you don't.  I'd rather get rid of the class initializer and document that you always have to call LibPD.Init() after loading the DLL, period.  Slightly less convenient but unambiguous.
This reads better if the last sentence is removed.
нужен пробел перед фигурной скобкой. все, что внутри if-а нужно сдвинуть на таб вправо
Previously value was being verified as being present before sending the method, is removing this check acceptable on line 27? Any test failures in AR with this?
Actually there is for running retest on inttest/. See [Makefile](https://github.com/rebar/rebar/blob/master/Makefile#L15) and [rebar.config.script](https://github.com/rebar/rebar/blob/master/rebar.config.script).
doc needs to be updated
I do not think it is wise for us to start using the case class copy method on Trees without a) wide agreement this is a good idea and b) somehow making it do the copyAttrs part automatically, which AFAIK we can't yet do. In the absence of both those, I think this should be
```
treeCopy.ArrayValue(rest, elemtpt, elem0 :: rest.elems)
```
like it is elsewhere.
Good question. This part of code was written by @odersky so I don't know what was the reasoning here. Strictly speaking `isParamAccessor` looks like this:
```
  def isParamAccessor    = hasFlag(PARAMACCESSOR)
```

so the contract we are talking about is not enforced. I hope @odersky can comment on this.
You're using `length` here because `count` returned an incorrect value, right?
Spaces after each comma, spaces around `+`.
First of all there is the `.det` method to calculate a determinant. Second, isn't it possible to have a determinant equal to one just by an accident while using the non-unimodular formalism. In such case this check will be wrong.
Agreed, the redirection is not needed. I will invoke tensor_product_simp directly.
your doctest has a clear=False, so a positive case should be clear=True, unless you plan on having multiple levels of true...
firstclass? empty?
There I go again, been awhile since I've had to manage memory manually. ARC has spoiled me.
There is unidecode in ````requirements.txt````:
````
...
SQLAlchemy==0.7.8
Sphinx==1.1.2
Unidecode==0.04.9
WebOb==1.2.2
WebTest==1.3.3
...
````
There's no existing tests using this particular style. They're all either single quoted with implicit concatenation, or triple-quoted multiline.
Just a question, would it make any sense to use the `folsom_sample_slide:new` API and store the slide record?  Or is this just a facade on slide? Excuse me, I have a general ignorance of these metrics.  I'm trying not to follow rabbit holes while reviewing this.
Dieses File ist persönlich... bitte nicht einchecken
Can this number be negative? uint perhaps?
The reason for the assertion is that it's impossible to have more than one cycle if you only have 3 nodes in the graph. If the calling convention changes we might have to make this more general.
No. They are dedundant.
Also, the author name is missing.. Pls add it later
Doesn't this take a path rather than an ID?  It looks like an implicit conversion is hiding the type mismatch
This doesn't actually have to return a Result<> if you don't want it to.  Result was originally a method of wrapping the format-agnostic payload "results" with some well-known schema (the timestamp).  With the way queries work now, we don't really need Result for new stuff (GroupByQuery doesn't use it).  If you prefer Result, that's fine too.
you say database here, but it's a collection below.
avoid use of the word "this"
omit read ahead.
Please call tr.next() once and assign the result to a meaningful variable name.
You have to take \n out with echo obviously.
Put this code in db tools perhaps?
Read the jQuery source code, then.
You'll see that it is (these are the minimum numbers, dunno really if 
there's more and with IE8 there's really a whole lot more) 8 DOM 
operations, 2 regex matches, 1 string parsing and 10 objects. To know 
the exact numbers I'd need to do more checks on how jQuery works for 
that specific operation, though.

Oh! And in IE8, jQuery navigates through all nodes in the DOM with that 
operation. So the min DOM operations is about 3 + the number of nodes in 
the document node. And I still don't really know what are the conditions 
that let jQuery use the querySelectorAll() method. There are conditions 
that it calculates such that makes it does not using that method. When 
that happens, 8 DOM operations is just a dream. The real number is much 
higher.

So, IMO doing that operation once per page load is completely acceptable 
due to how much it simplifies our code but doing it more than that is abuse.
Whoops, it is alternative, not old: https://developer.mozilla.org/en-US/docs/DOM/Storage#localStorage

> Storage is defined by the WhatWG Storage Interface as this:

>	interface Storage {
>	  readonly attribute unsigned long length;
>	  [IndexGetter] DOMString key(in unsigned long index);
>	  [NameGetter] DOMString getItem(in DOMString key);
>	  [NameSetter] void setItem(in DOMString key, in DOMString data);
>	  [NameDeleter] void removeItem(in DOMString key);
>	  void clear();
>	};
Actually as this is in a test, there is no need to use either, you can just hardcode the path to the correct location, and $phpEx is always ".php".
Should have a trailing semicolon here.
If I recall correctly, $lang can be a non-string, so before he was doing something like $user->lang((string) $lang); and I suggested just wrapping in quotes, as that automatically converts to string and is shorter. If how he was doing it is better, then we can go back to that; otherwise, we should leave it as is to ensure that it is a string.
may as well add more linter pain. +1
I think it's completely reasonable use-case to have multiple perf marks/timelines with the same [group,label,id], so I don't think this should be a warning.
Maybe -1 instead to differentiate it from EFFECT/ZERO.
I suppose Jan made that patch a while ago before 3.2.21 was available. I could try to update the patch, but I'd prefer someone do that who actually uses Xen and has a way to test the result.
move this handler to the controller?
Don't need these comments.
You can't call x.shape.eval() safely. Most of the time, this will fail, as this will need inputs to know the shape of x.

As this is only used to determine the output type, you can set the output variable like this I think:
theano.sparse.csc_matrix(dtype=x.dtype)
nvcc will pass the appropriate version of -mX to the compiler anyway.  No need to specify it ourselves a second time.  And cl.exe (on windows) would not accept -m32 as a valid flag anyway.
I did this mostly because it was easy in the beginning,  so I don't have a problem changing it. Perhaps though, we should move the gpu and 1d optimizations into AdvancedSubtensor or make them proper optimizations?
``functionalities``
I think ``were tried`` was more accurate. Maybe remove past tense: ``DebugMode checks all optimizations being tried, even if they are rejected``.
You can actually use the same syntax with ``theano-nose``. I don't think there's any advantage of using nosetests (not 100% sure though).
In a perfect world, replica would be a START_ACTION; it uniquely identifies a start mode.
I'm not seeing a better alternative here given the speed of the == tests and the unique handling required in each specific case.
I feel like these two methods need comments explaining what they do and how they are different.
Use %r instead of %s when logging, otherwise it will fail on non-ascii characters.
`console.log();`
Yeah, looking further, it appears this idiom is used a few more times, which to me triggers Fowler's "Third Time Refactor" rule. I think pushing this logic down into a manager method is going to lead to cleaner code (and easier-to-write code for end-users as well).
Test cases are isolated, objects created in one aren't visible to another.
Spacing issue?
Just because it's not the source, doesn't mean it shouldn't follow the style guide LN1-7
This deserves a retest on Safari 6.0, as it might be present there, and I'm fairly certain it's fixed in Chrome.
Should use the entire height of listbox, not just its body.
Make sense to either remove this Makefile or make generic the python version include?
Again with multi-level comprehension, could that be avoided?
Right, but isn't that functionally already what happens when we autocomplete the url?
Shouldn't hardcode `/usr/local` since some users use nonstandard prefixes. The HOMEBREW_PREFIX variable has the installed prefix.
As I needed to rebase this formula to the latest version of fontforge.rb anyway, and didn't want to potentially make a complete mess of things in the process (ah, rebasing...), I'm resetting this pull request to the newer branch where I fixed this: https://github.com/codeman38/homebrew/tree/fontforge-20120731
Damn right you are.  First question - how often do these failures happen, and what kind of failures go into this block?  Is this a 10% failure block, 5%, only on connection issues?    If it's <1% rescue and reraise (rather than throwing something specific).  If it's more than that, the logic in the block is questionable.  For instance, it's eating the exception message which means we have no way to debug this if a user hits it.  Why not always include the message?

Needs to be raising a specific exception, not RHC::Exception (RHC::PortForwardFailed)
Do we need "the reddit server"? We've agreed to remove 'the reddits' above and that we are making a request to a server should be self-obvious.
Ha ha !
Besseren bezeichner bitte
Maybe `VLength` or `VPrintable`?
need a space after that comma!
We should get the warning that RADMON was missing from the top level get_file (though that doesn't actually check to see if the file is *readable*).  Also, the get_file warnings don't seem to end up in the processing summary... another thing to fix.   In this part of the code, I'll just do an open file successfully or return(undef, undef) from the function.
why final ?
Maybe this should be buffer2.size() compared with buffer.size() instead of 10U?
Don't delete things that are not there - i.e. don't include code that is not necessary
Minor note, I wonder what the performance of this is like. Merging two arrays (at least one of which will be quite large) I expect has a cost to it. How about restructuring it like so:

```php
<?php
//...
if (isset($this->_routes[$name]))
    return $this->_routes[$name];

if (isset($this->_customRoutes[$name]))
    return $this->_routes[$name];

throw new LookupException('No route named '$name'");
```
@dsaff 
Should be this constructor removed now?
thx.
I try to always preserve/use the name the service/api calls the data type natively, since we map it to 'self' regardless... and 37signals may have other things they call profile or user.
may as well remove this line if you're overriding it in the next line.
Should also remove the require for UrlParams.
Is this not an event handler? It is in node.
Yeah so I put both there because I use the minified one in code and the
readable one for source diving. Mike's probably right that the minifying
provides little benefit vs the gzipping we do.

On Dec 17, 2012 8:55 PM, "Mike White" <notifications@github.com> wrote:

In
corehq/apps/hqwebapp/static/hqwebapp/javascripts/diff_match_patch_uncompressed.js:

> @@ -0,0 +1,...

+1 for only including a non-minified version.

Minified versions suck for code diving, are going to end up making me write
a bash function wrapper for grep so I don't have to look up the flag for
truncating long lines, and probably aren't going to have a huge impact on
performance. If you can afford to worry about the performance benefit of
removing a low fraction of the size of your JS files (they're probably
gzipped when sent anyway), then you can probably afford to set up an asset
pipeline that abstracts it away for you and bundles groups of files that
are used together, which is really half the benefit of minification.



—
Reply to this email directly or view it on GitHub.

[image]
I would suggest keeping the consistency in the attribute names and make sure we use PEP8 as much as we can. 

You should rename dtTrade to dt_trade. Also, I would probably use trade_date even if longer than dt_trade.

What stands for 'df' ?
you don't have to add "final" to primitive method arguments.
I tried taking out this line and putting this at the top of `locate_grid()`, to the same effect as what you suggested:

```R
  if (empty(data))
    return(panels)
```

But it fails on this test, I think because it later looks for a variable that's not part of the `panels` data frame:

```R
df0 <- data.frame(mpg=numeric(0), wt=numeric(0), am=numeric(0), cyl=numeric(0))
ggplot(mtcars, aes(x=mpg, y=wt)) + geom_point() + geom_point(data=df0) + facet_grid(am ~ cyl)
# Error in eval(expr, envir, enclos) : object 'wt' not found
#
# Enter a frame number, or 0 to exit   
#
#  1: print(list(data = list(mpg = c(21, 21, 22.8, 21.4, 18.7, 18.1, 14.3, 24.4, 
#  2: print.ggplot(list(data = list(mpg = c(21, 21, 22.8, 21.4, 18.7, 18.1, 14.3,
#  3: plot-render.r#171: ggplot_build(x)
#  4: plot-build.r#39: dlapply(function(d, p) p$compute_aesthetics(d, plot))
#  5: plot-build.r#26: f(d = data[[i]], p = layers[[i]])
#  6: plot-build.r#26: p$compute_aesthetics(d, plot)
#  7: plot-build.r#26: get(x, envir = this, inherits = inh)(this, ...)
#  8: layer.r#159: compact(eval.quoted(aesthetics, data, plot$plot_env))
#  9: layer.r#159: Filter(Negate(is.null), l)
# 10: layer.r#159: sapply(x, f)
# 11: layer.r#159: lapply(X = X, FUN = FUN, ...)
# 12: layer.r#159: eval.quoted(aesthetics, data, plot$plot_env)
# 13: lapply(exprs, eval, envir = envir, enclos = enclos)
# 14: FUN(X[[2]], ...)
# 15: eval(expr, envir, enclos)

```
Please fix this that there is some unnecessary space in the register text (both on button and on msgid). You see? Maybe this is the reason. And reset button does not have translatable label but is left to the default.
Add an empty line before load.
So you do know how to do this. So do it also for logout.
Extra space at end, will miss geocode cache.
I don't think we can do that as it doesn't return what we want.

    var a = new Date( 0 );
    // returns 19 for me
    a.getHours();

    var b = new Date( 0, 0, 0 );
    // returns 0 for me
    b.getHours();
It looks pretty good actually
Since this is on mousedown it looks like we never get a chance to get the values of `dragging` and `resizing`, which kinda sucks.
To solve this situation https://github.com/mozilla/butter/pull/808/files#L2R67, we can just omit this declaration, right? Why is `butter-btn` declared here with this level of specificity? Does it need to be?

Since it's already defined inside of `css/buttons.less`, we can probably just avoid this altogether and live happy lives.
Classname that sets everything for new popup images is good
you should be able to delete all session keys with just this one line:

`req.session.reset();`
Yes, you are right. Please add this as a separate commit, since it's not related to the Python 3 support.
burst must not be zero.
Borschik — это расширяемый сборщик файлов текстовых форматов. Его основной задачей является сборка статических файлов веб-проектов (CSS, JS и т.д.).
If there's no callback, add the callback provided by Q
Maybe overkill? We're still prerelease.
Should defining the name syntax be an api on the model so that same algorithm is used consistently?

Is there a reason that we use view label vs view name in the syntax?
If we use view label, should we also use env label for consistency?

Note: since the changeset name is only used on katello (and not in backend systems), using name (which may contain UTF8) is probably ok.
Christine and me have discussed and found some situations where flash.keep is necessary, I'll add it in.
This site advises against that method:
http://vatuma.com/tutorials-tips-and-tricks/for-developers/creating-dynamic-css-for-wp-theme.html
Recommends an in-line style sheet in the head.
please remove the extra empty lines
@stof That's why I asked. :smiley:
This still has a race, I'd setup the command inside the async void method. If nothing is awaiting ```InitializeConnection()``` then there's a small window where the connection isn't initialized yet.
I modified the algorithm and limit the sentences returned to 1, which reduced the complexity from O(2^N) to O(N^2). see line 2189:
```
  this.getSentence = function imedb_getSentence(syllables, callback) .
```
My idea was that if there's more than one .rst file, the directory definitely makes sense.  But the use of ``:automodapi:`` here removes the need for distinct ``api_*.rst`` files, so right now only one file is needed.  If in the future someone wants to make a separate page, then it should be moved into its own directory.

This point should definitely be documented somewhere, though - where do you think it goes... The documentation guidelines, the subpackage template, or a commented area in the .rst files?
!define PRODUCT_VERSION "${VERSION}.0"  ... would save us a little typing and another potential source of bugs.
We don't want ordering. What we want is that services relying on the network being up if they depend on ``network.target`` to actually have the network, including cjdns, to their availability. As we discussed in IRC, one could imagine some server service which wants to bind some socket or has to do some DNS lookup at startup. Imagine you configured that service (let's call it Apache for the sake of convenience) to use a HypeDNS hostname, which implies it has to do a HypeDNS lookup at startup. If cjdns isn't up at that time (i.e. when the service starts, which might be right after ``network.target`` has been reached and before cjdns has finished configuring itself, it's all racey), that lookup will fail for some reasons, which is bad.
Missing space around `!`.
otherwise you create useless spaces
I wasn't sure how much I wanted to tie the 'QtFoo'-style objects to Qt with signals and slots, since they don't inherit from QObject, but Python object, so I felt Python conventions were more appropriate.
Indentation weird here. you probably want:

    station = {
        ...
        corp = {
            'id': int(row.attrib['corporationID']),
            'name': row.attrib['corporationName']
        }
    }
You can just use `stop_for_status` instead
If you want to delete them, don't just comment them out :)
yeah, didn't feel right for some reason...
OK I missed that.  Sounds good.
Shouldn't you just use `$parameters` here?
yeah. much of l10n_utils is under tested :(
Fix alignment please.
Why use uncached here?
Nit: trailing semicolon.
"then you can"?

Also, please add a newline at the end of all your files. That one is missing one. :)
Can this not be removed? It's not part of `Exception` so it'll be raised and captured later on line 698 and here it doesn't add any value.
On my machine when i am using the openjdk-7 (or 6) compiler detect error what do not access to private field StoreDir in Script type from it's child.

I can test this once more.
I Agree, this do not relevant this issue.

Should create a new issue to check it out?
If it is a filter, shouldn't it be Elastica_Filter_Abstract? Please always use { } also for 1 if / else clauses.
No protected property $_params is only used for storing script params, lang and script are stored in private properties $_lang and $_script;
So if no script params were set, what the point of adding them to output.
Inline the expected_items there.
That would be nice... If the Sculpin class implemented ContainerAware, would we just need a getContainer() function to retrieve it externally? Thanks!
This can be simplified a bit with `Hash#fetch`:

``` ruby
definitions.fetch('message_type', DEFAULT_MESSAGE_TYPE)
```
I did not :) This makes sense. Please add a comment then that has at least the length of your explanatory comment above ;)
Should be:  `if (1 === Version::compare('2.2.0')) {`
OK, I also added the missing blank line to all other docBlocks
@vicb this would fail as it would try to call the ``closure`` method on the class. You would have to assign it first :)
phpcs includes PSR-1 and PSR-2 since 1.4 and 
>Symfony follows the standards defined in the PSR-0, PSR-1 and PSR-2 documents.
It is more readable to do:
self.dimView.alpha = sender.on ? 0 : 1;
Yes, you are of course correct. I'll get this sorted. In the meantime, I've reverted the v6.git changes I made.
Good point. The single responsibility principal definitely trumps number of lines. If you have a method doing multiple things I agree number of lines becomes irrelevant and refactoring is a must. In this particular instance I think all of the stuff in the create is consistent with creating. I could see an argument that populating the flash might be something different, but I can also see a counter to that.

As for well named private methods my only thought there is that names are subjective. A perfectly sane name to Person A can look completely bonkers to Person B. Code is definite.  For instance:

     params[:my_object][:my_property] - is clearly pulling the property values out of the object hash out of the params hash
     
     def get_my_property_from_my_object - could be doing any number of things. It could be getting a value from a db, or making a web service call. I'll have to go look at the method to be sure.

Obviously this is just an example and not a great one by any means, but I do think it's worth considering. Another thing to keep in mind, and this isn't a major deal, but in a lot of cases creating a one line private method just turns one line of code into four.
图表混排的那个caption还是比较难处理，我也没怎么深究。如果加“@”编译没问题的话这儿就先这样吧。
Does StandardError work with python 3.x too?
(If not, why don't we just catch "Exception", since we just log and exit, it's not a problem if we catch system.exit())
Should this be an extensions test?
For completeness, it probably makes sense to return `r` from the wrapper function.
this shouldn't be global, it should be in `main()`
am going to recommend we unilaterally upgrade to ES v0.20 so most if not all this stuff can be removed.
Good idea! I tried the new editor area in geocanopy two days ago, and one of the things I wanted to do is have other actions in the empty widget.
What is "action", more documentation on this would be nice?
Copied this from ItemsController, but I hate it. It needs pulled out into the model or its own class.
testing the default/opposite would be a nice touch :)
This line compiles to:

````javascript
if (this.show_default_text_always(this.search_field.val(this.default_text))) {} else {
  this.search_field.val("");
}
````
Which is not what you're after. In coffeescript, you need a then after the if's conditional.
блин это github тег схавал, я имелл виду убери тег BR, он в HEAD не нужен
these are important, they show whether it's sudo or not
Should be `min-height:64px`? The message could be long over 3 lines...
`AlarmEditView`?
Now you can use appsInDB and remove the comment
here's the p inside the label...
I’m really surprised that inserting a block-level element into an inline-level one is valid. Is there no other way?
Why did you need to do that?
Could you explain what 'calendar app' is? Is it part of the calendar (like week or day views) or instances of the calendar (can we have more than one)?
Object.keys! What a cool hack ;)
Why not use classList.toggle instead adding and removing?
If you remove the exceptions loop, you can probably simplify this functions to be return installedApps.slice(0);
I thought it was ugly when "read more" was split. The design calls for it to not necessarily be on its own line.
revert to new syntax ```only: [:new]```
Can we rename this to CountMetric?
Let's get rid of this
Why is it that 30secs tuple timeouts can be tested with advance-cluster-time without synchronization?  

With global semaphore implementation, what happens if metrics tuple is not delivered, unit tests hang? My logic: If you decrement a semaphore when it's at 0, you block, until someone increments it (it's incremented when the tuple is delivered)

Here's another solution: `advance-cluster-time blocks` until all the time is simulated AND all queues are empty, all tuples were processed and acked, and IBolt::Execute functions returned.
yeah, but given that we spin up a process, it is nice to collapse these things so we don't have to run multiple procs.
"MIT license" (c missing)

"more information" (there is no plural "s" for information)
Basically, you add `--drb`to the command line to run via spork. That's it. AKAIK, `drb` is only ever used w/ spork and `drb_port` is never used (well, it's used internally, but never by a user).
Also this indent doesn't match
Yes, could remove this, as it is effectively zero anyway I think?  I thought I should add a default value in the options as it makes it explicit what z-index the tile layers will have by default.
Could you please fix those sprintf with snprintf's? Thanks!
Needs a more descriptive description.
Say that the there's new archive search links in the menu now.
Rather than a pass, this should probably be written as an `if not issue`
Not necessary here. You can just use forward declaration of `RegisterQAppAssociation` and include it in mainapplication.cpp
The formatting of this hash feels a little awkward.
For this fallover use-case, do we have a plan on how we handle statefull
schedulers?
I can only think of Timed but there must be some schedulers that have state
maintained in their own class attributes. Those might be different from
master to master

Pierre


On Tue, Oct 30, 2012 at 3:24 AM, Dustin J. Mitchell <
notifications@github.com> wrote:

> In README.md:
>
> > @@ -40,6 +40,16 @@ The outstanding resource types are:
> >  * logfile
> >  * buildslave
> >
> > +### Schedulers ###
> > +
> > +There is no scheduler resource type yet, although schedulers are in place in the database API so that other tables can refer to a scheduler ID.
> > +Support needs to be added for schedulers on a particular master to "claim" the name for themselves, deactivate themselves if already running on another master, and periodically poll for the opportunity to pick up the role.
> > +This gives us automatic failover for schedulers in a multi-master configuration, and also means that all masters can run with identical configurations, which may make configuration management easier.
>
> Right, both of these issues need more thought. Regarding schedulers that
> run on all masters -- good point, and I think that will differ by class,
> rather than as a configuration option. Regarding load balancing and
> failover, I think we might want to have a configured "priority" that allows
> schedulers to prefer a particular master. Then users can either configure
> all schedulers to prefer master A but fail over to B, or configure each
> scheduler to prefer a different master.
>
> I'd like to figure out a way to leave this open for later optimization, in
> the short term.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/buildbot/buildbot/pull/551/files#r1974609>.
>
>
sch.name is internal name. Not sure it is a good idea to show this to the user
you should not add your IDE files to the gitignore of the project but ignore them locally (and potentially system-wide to avoid doing it for all your projects): http://help.github.com/ignore-files/
These changes make no sense, infoCache is considered safe, and in any case this should be $identifier and not the escaped version of it.
why making a separate method for each type ? Simply use the DownloadManager which is responsible to handle the different downloaders
`commit = user_project.commit ref`
Creating project in admin area should not automatically add admin user to masters
re: TODO's - either do it now or remove it.

We've not got to editions yet, so we'll address that once we get to that feature set.
& => &amp;, if outputting the raw value in the template
Galleries are not really associated to articles, probably want to word this as...

Given I visit a gallery
Proposing
```C
   /* There is no specific field that we can check to prove that the packet
     * is a Teredo packet. We've zapped here all the possible Teredo header
     * and we should have an IPv6 packet at the start pointer.
     * We then can only do two checks before sending the encapsulated packets
     * to decoding:
     *  - The packet has a protocol version which is IPv6.
     *  - The IPv6 length of the packet match what remain in buffer.
     */
    if (IP_GET_RAW_VER(start) == 6) {
```
didn't know this notation, interesting
I'm confused. Does this apply to toggles, checkboxes, or both?
It's not clear what this change does or why is it necessary, could you elaborate ?
We aren't using one-liner. Can you move down `test.done()`?
Maybe we could just call this `window/browser` instead.
Shouldn't: `it's no way` be `it's in no way` ? (I'm not sure)
@warner, @lloyd - I like this approach - where else do we have to worry about this?

One cleanup that would be useful for cohesion is to move the requiredEmail check from dialog/resources/state.js->"start" handler to dialog.js with these so all inputs are checked in a single place.
This should be attached as a separate pull request. Please remove it from that one.
If puppet is not able to run the catalog (typo in my manifest, whatever) will puppet send a report in this case? In my opinion it should but looking at the code I guess it does not.
关闭文件？
Looks like MRI does return an instance of time. I added a test to this effect.
How about:

`it "returns nil if the argument provides neither #to_str nor #<=>" do`
> because, you know, I'm ruby people ;-)

:-)

Can you merge this? Or should I wait for brixen (or someone else) approval?
Thanks -- fixed.

On May 22, 2012, at 3:16 PM, Public Keating wrote:

>> @@ -629,6 +629,18 @@ TodosThree.TodoItemView = SC.CheckboxView.design({
>> });
>> </javascript>
>> 
>> +This file is not used in the app, but is here for illustration of how you could pull in code from an external
>> +source file. If you look at resources/main_page.js, you will see that the TodosListView has exampleView set
>> +as "inline" code for the same checkbox view you see in this resouces/todo_item.js file. Instead of defining
>> +the exampleView there as and "inline" code block, the require that is commented out at the top of 
> 
> there's a grammatical mistake ..as **and** "inline"..
> 
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/sproutcore/guides/pull/120/files#r863425
Here I would catch both exceptions as described in my [previous comment](https://github.com/tobami/littlechef/pull/106/files#r1654540)
Break this into 4 discrete test methods:

- testReturnsOneOhVersionWhenDetectedInServerSuperglobal()
- testReturnsOneOneVersionWhenDetectedInServerSuperglobal()
- testFallsBackToVersionOneOhWhenServerSuperglobalVersionIsNotRecognized()
- testFallsBackToVersionOneOhWhenNoVersionDetectedInServerSuperglobal()

Additionally, add a test indicating that if the version is explicitly set, that version is used.
I suggest put this block before the above check and simplify the complexity of the conditional sentence
Bugfix: Filters shouldn't be run during merge?
Why remove this?

Because is the last assert in the test case you can refactor this try{}catch as $this->setExpectedException
You are right, i changed to return the SplPriorityQueue. I'm also thinking to add a setWriters() to offer a full interaction with the queue of writers.
Do not prefix non-public members with an underscore. We only allow it in the CS for legacy reasons of the old codebase; new code should not have them.
Unnecessary local variable
Though perhaps obvious, the docstring ought to be updated.
Mink 1.4 is incompatible with Behat prior 2.4, BehatBundle and MinkBundle
tnx, good catch!
!defined is a bit of an ugly thing to use, but for once I think here it might make sense. Maybe it would be possible to use a virtual resource instead?
Yes, it breaks dot notation and converts it to bracket notation for the form.
Guys,

Ye are including the wrong 'Windwaker' on this thread. I think ye are
looking for 'W*1*ndwaker'

On Tue, Jun 12, 2012 at 3:58 PM, ZNickq <
reply@reply.github.com
> wrote:

> > @@ -186,7 +186,7 @@ private void setupWorlds() {
> >               if (WorldConfiguration.END_LOAD.getBoolean()) {
> >                       TheEndGenerator endGen = new TheEndGenerator();
> >                       World end =
> game.loadWorld(WorldConfiguration.END_NAME.getString(), endGen);
> > -                     end.getDataMap().put(VanillaData.GAMEMODE,
> GameMode.valueOf(WorldConfiguration.END_GAMEMODE.getString().toUpperCase()));
> > +                     end.getDataMap().put(VanillaData.GAMEMODE,
> GameMode.getByName(WorldConfiguration.END_GAMEMODE.getString().toUpperCase()));
>
> Well, it's used right here...o.O
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/VanillaDev/Vanilla/pull/237/files#r967514
>
the plugins stuff should have the same version as akiban server if possible
> I'm not sure, but couldn't you have used operators.attrgetter?

I've looked at it but I couldn't find a way to use both attrgetter and
itemgetter together.
Quite common ? in bug reports yes :)
Could you add some doc on the following facts:
* That this param signifies difference between a create new and create draft
* That this is temporary measure until createDraft() is fixed to not reuse create function
I'd rather have the comment above the index creation, it looks a bit messy this way.
rename to os_putenv_re (to avoid confusion)
When connect_confirm is false we go directly to if( $connectConfirm ) { /*...*/ } block which ends with return $this->container->get('templating')->renderResponse('HWIOAuthBundle:Connect:connect_success.html.twig' ... - the confirmation step is skipped.
When connect_confirm is true we either check the form ($doConnect = $form->isValid()) or return the confirmation form(depending on whether the POST or GET request is being processed)
Why move this into base.py ? It will be different on every machine.
Yes: http://nodejs.org/api/fs.html#fs_fs_readfile_filename_encoding_callback
Not a complete rewrite, again i merged everyone else's commits into one
I moved them into the qunit-fixture div, and it failed worse, so moved them out again.
So, this doesn't matter so much in the tests, but you could simplify all this by using the `this.openTestDatabase(function (db) {...` stuff, it'll then take care of the closing & cleanup for you. The tests that don't use this have a specific reason to avoid it so your test can focus on the important stuff instead of managing the db.
when i tested, i needed 0.13 for 1.9.. maybe I missed something, I'll test again
instead of checking for 302, do something like

```
eq_(response['location'], settings.LOGIN_URL)
```
or something like that. 302s can happen just because of various redirects for legacy or for adding or removing a '/'.
Using factories works for test purposes but a production application would still require the rescue. This was the reason I added the find_or_create_by method to clients. In most of my use cases for this gem, the intent is to either find or create a client/user/project and add to it invoices/tasks/etc. Harvest doesn't provide a method for finding these by name. I do not believe adding something like "harvest_client_id" to an application is a better approach.

I'm happy to discuss a better method, but it needs to be accessible to the application using the gem, not just for testing.
Yeah, I guess i could use newlines instead of an extra ```puts```
you’re leaking “registrators” since you didn’t call super.preRestart()
As above... having a module allows a whole lot of additional flexibility
Related: should `Pods` be changed to `config.project_pods_root.basename` in [this line](https://github.com/CocoaPods/CocoaPods/blob/56e8c133e2460df89a3da8de5b7ec40c45210c96/lib/cocoapods/podfile.rb#L75)?
In general, I have been wondering if we needed this file given how the sprites file was restructured.
有wifi的情况下，也有进入wifi设置的需求。如：切换wifi等 。建议点击wifi icon就可以进入wifi设置。
this is a required attribute
For cucumber tests, we want to test the happy-path of our code. These tests are better left to testing using RSpec. You should remove all the cucumber tests that look for errors and use rspec to test those cases.
Not at this particular line, I've added few more lines to ``Injector`` documentation which have this information now.
Mmmm.... this makes me think there is actually an error here.
CheckParams should return true here.
I'll fix it and update the pull request
I'm not sure what you're trying to do here, but the parentheses and quotes don't seem to match.
Generally you want to split up whitespace changes and comment deletions into their own PR so they don't clutter up your original PR