labofsahil/github-event-dataset-2012 huggingface dataset:
Yeah, still working on it, but nothing existed before. There's (finally)
pretty good coverage unit-wise. I'll fill these out at some point.



On Thu, Nov 1, 2012 at 1:52 PM, Camilo Lopez <notifications@github.com>wrote:

> In spec/integration_spec.rb:
>
> > @@ -0,0 +1,15 @@
> > +require './lib/multi_db'
> > +
> > +describe MultiDb do
> > +
>
> Did you mean to leave all these guys pending ?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/burke/multi_db/pull/3/files#r2008545>.
>
>
I'm trying to resist bikeshedding this conditional, `x && x.` always bugs me, but I can't think of something I like better.
commenting here (already discussed on skype) re: naked get_db() calls. If we're going to go the route of splitting the db up eventually (either type or domain), typing out the get_db() like CommCareUser.get_db() seems more preferable here.
doesn't necessarily break an abstraction barrier (since pillows.py is in the same repo), but get_es().get(CouchPillow.es_index, CouchPillow.es_type) might be one way to reduce nastiness - or make a mixin for the commcarecase class?
(and edition is a release of a book)
ow, I may have undestood. If you clear the filter, all the lines will become visible again. I'm right ?
Actually, why is it runnable anyway?
前面有一個叫 `topbar`
這個就叫 `subbar` 吧
Strange indentation. Do not break the line before and it will be OK.
`clean`: https://docs.djangoproject.com/en/dev/ref/forms/fields/#django.forms.Field.clean

So you should return the value, if everything is OK.
OK, Facebook makes a redirect, so it is a GET.
Check only limits if they are defined. Does `date()` work both on `datetime.datetime` and `datetime.date` objects?
Move this style block inside head vs. body, it's not valid here.
I don't think sliders would be worthwhile, since Firefox doesn't support that element type (range).
Kind of sucks that you still have to work with innerHTML here.  Should we provide some nicer way to not have to work directly with a dom node?
Why no space here after the comment slashes here, but in other spots?
Can you be more specific? Do you mean a new line for each declared variable?

This section has a number of declared variables further down. Is it worth taking this opportunity to move them all up to the top?
if this is undefined, what happens?
the default should be your option for the value, IE <code>default</code>
will this be fine once it gets hosted somewhere?
unrelated change - revert back (same for most changes below)
I'm not super familiar with those api's, but I think they both do rate limiting and will occasionally send back 500's (or some variant that's more meaningful). I guess if you get no data back you'll just die silently, right?
`/`, not `./`
Message updated as no native speakers around here it seems....
That's good to know. For this page, both are equivalent though, at least, I believe.
Please specify a specific commit when there are no tags yet:

```ruby
s.source   = { :git => 'git://github.com/ZaBlanc/RaptureXML.git', :commit => '5c81438cb88c6040608eb00d8aa63f1763e1d025' }
```
Usage needs change
Change the output to 'To be merged files'
Sadly this file won't work on Windows, maybe use a file in the working directory, deleted with `scope(exit)`?
Need to fence here to allow :create in katello-mode
If we no longer need the 'new' action, we can remove it from routes.rb.
will see if the action can be included app controller; however, need to ensure that the permissions can be controlled by the specific controller where the action is executed from
how would you take care of the level?
Sort & Remove namespaces kthxbye
This method shouldn't really be called `get_container_cdn_url` because it just returns an internal path to the object.

` get_container_cdn_url` is supposed to return a full CDN url of a container. In AWS CDN is handled by a separate service (CloudFront) so adding CDN support is more involved.
Do you mean "logging file" rather than "configuration file"?
Ok - so for the ID stuff, shall I add the id argument to ``from_table`` for now since I think that is what you were suggesting before? (unless I misunderstood). Or should we just require the user to set ``t.meta['ID']`` before writing to a file if they want to set a custom ID?
Added a printf("WARNING: Upgrading directly from shared to exclusive mutex, something is wrong here: %s:%d\n" if we upgrade directly from a shared to an exclusive lock.
By the way: yes, it's ugly - I'd prefer to either get rid of v2 altogether (but that would break support with boost <1.44, which is apparently still used in some placed), or get rid of the conversions to C strings entirely. But that's probably not for anytime soon, as even boost's own lockfile implementation requires a const char* argument.
It's our standards that we decouple logical blocks by a line break.
Assignment statements, conditional statements, method call statements and loop statements are always separate by a line break except if they're the first or the last in scope.
Please add week tests for Russian locale.
Week will start with Monday ( weekday == 2 ).

This should protect us from some assumptions like "ALWAYS : weekday == 1"
I'm not an SEO-expert, but I think this is something wierd. The canonical url is the url that contains the original content, so if you specify an url without the language the first thing Google will see is a redirect, and it will follow this redirect so the endresult will be the url with the default language included.
I think the identing level is slightly off
Did you mean to have a `urljoin` here?
Code style comments.  Put spaces before and after `:` in each of these dict elements.  Spacing of callback to be `function () {`.
consider constructor param Ticker as it is testable, whereas hard-coding currentTimeMillis is not
I've just looked again into sources and realized that we don't need to have special case for e != ee, but it should improve performance of code. Let me clarify. If I clearly understood, only DefaultWeightedEdge should be edge class provider, the code
```java
public double getEdgeWeight(E e)
    {
        if (e instanceof DefaultWeightedEdge) {
            return ((DefaultWeightedEdge) e).getWeight();
        } else {
            return WeightedGraph.DEFAULT_EDGE_WEIGHT;
        }
    }
```
shows us, that we don't need tests identical edges, since it is the same object.
That's true. For some reason I merged this pull with a previous one... Don't know how to separate them
Meeeeh. It'd make things much easier ABI wise if we could agree on freezing this.
Hrmmm. So, I guess it makes sense that it's not in config.js because config.js is used for run-time configuration, but I really don't think it makes sense as being a konf file. It follows the same (function(Mobify){})() pattern as the rest of the api files. I would argue that it is either a js file that lives inside of api/, or it is just something we should add to default.konf. 

I dont see what's wrong with adding that data into the Mobify.conf object, can't you just grab that variable when you need it inside of the resizeImage function? Default.konf already contains compile-time configuration variables like {build_dt}.
Could you add the asterisks with CSS? Then the asterisks could be changed for RTL languages or locales that don't commonly use asterisks for this kind've thing.
Nice testing!
It's neat to know there's a setnx, though. I was totally looking for that, but assumed (incorrectly) that all the set* things differed in the value type.
I like that copy, haha
Move the `if percentage is None: return` statement up to here to not have to go through all the conditions.
do you need to worry about `'.'` or `'..'`?
Why don't we close this window here anymore?
you don't need to check arguments.length here, typeof is enough
Why `over` - typo?
Typo of regi<b>s</b>try?
Symfony uses ``integer`` in the phpdoc, not ``int``
it's '' now, not null
It was fixed recently in [`2.1`](https://github.com/symfony/symfony/blob/2.1/src/Symfony/Component/BrowserKit/.gitattributes) but seems like not yet merged into `master`.
Why do we need a cache layer? Is it really worth it?
@raziel057 if you use the way suggested by @bschussek, there is a high chance @fabpot would simply change it in a "Fixed CS" or "Tweaked previous merge" commit after merging the PR :)
I kept this from the previous code. What is the implication of the "is_safe" setting?
What hav you changed `.` with `! `? Is it on purpose? Does it have to be like that un Hungarian?
extra space here (running the PHP-CS-Fixer would fix it)
Hi Amin,

I'm sorry about the delay. I had a test today. There's a little bug I'm trying to find. I will get this fix before tomorrow.

Sincerely,

Di Huynh
Computer Science and Mathematics
The University of Texas at Austin


________________________________
 From: amshali <reply@reply.github.com>
To: Di-Huynh <di.huynh179@yahoo.com> 
Sent: Tuesday, March 20, 2012 7:10 PM
Subject: Re: [OpenBook] Comment class (#28)
 
> 
> import play.db.jpa.*;
> 
> @Entity
> -public class Comment extends Model {
> +public class Comment extends Commentable {

did you fix this?

--- 
Reply to this email directly or view it on GitHub:
https://github.com/w7cook/OpenBook/pull/28/files#r583306
weak, nonatomic
moved to new line but the block didn't move on auto-format
thanks for all these places-> venues updates.
on these last two changes, i would have brought the whole if statement down to the second line..
Might be nice to extract this out to be useful across the board (not just matplotlib, not just features...).
Cool. Would you mind mentioning how most people create one of these (i.e. with ```pyplot.axes```/```pyplot.subplot```)?
Have an explicit option -- params["strategy"] or whatever: "sequential", "semiparallel", "parallel", etc.
clean up formatting of file
I agree in principle, but I think that as the first sentence of the introduction, it is permissible because it serves to motivate the study of HTTP.

I will factor out the "before" next time I'm working on the paper.
Not to worried about it.  Whatever license lets people do whatever they want with it.
Tu veux énerver les agilistes en empêchant d'envoyer des mails ou quoi?
what is RequestsIntegrationTest ?
The `nokogiri` dependency can be removed as soon as a new version of `github_api` is released, see peter-murach/github#23
We're not using `linkifyTest` or `linkify` anywhere.
Revert this one.
What typo does this catch?
I’d probably “namespace” this with a `Log` object:

```javascript
var Log = (function logjs() {
  // ...
  return {
    error: function(str) { log(str, 'error') },
    success: function(str) { log(str, 'success') },
    warn: function(str) { log(str, 'warn') },
    info: function(str) { log(str, 'info') }
  };
})();
```
Add line breaks between your block of rules.
{
      border: 0;
    }
This transitionend will never happen
As far as my understanding, the "choice" option is no longer needed... :-O Is it?

BTW, do you mean the code regarding the Contacts.load() or the Recents.stopUpdatingDates() ?
Oh noes! The kittens!

No way around `!important`?
Shouldn't this be called before deleting the messages? We want to show the screen while we delete the messages, not after, even when the user only will notice anything when we repaint.
remove this line, the variable is useless.
defaultImage.textContent = result.metadata.title ? result.metadata.title
                                                       : navigator.mozL10n.get('unknownTitle');
double quotes ! oh the horror ! ;)
I have been rethinking this for a long time and I would say no. But of course everything is improvable... Maybe in a future pull request? :p
So what do you suggest is to use this?:
<pre>
&lt;button&gt;Default&lt;/button&gt;
&lt;button type="submit" class="accept"&gt;Blue one&lt;/button&gt;
&lt;button type="reset" class="dismiss"&gt;Red one&lt;/button&gt;
</pre>
I have assigned id="alarm-item" for each alarm items in alarmlist.
So, I need to check it.
Let's remove the `label` and `menuLabel` of these IME specific layouts then.
This workaround is hiding the real problem of Keyboard. You coud check in all apps (there are more scenarios described in bugzilla) that this bug is easily reproducible yet.
Will do / I probably need license blocks for sax.js and caldav.js anyway.
Wow, I had no idea Ruby had this behavior previously. Thanks for investigating this, @soulcutter.

What do you think about this?

```ruby
task_block.call(*[self, task_args].slice(0, task_block.arity))
```
Breaks it since I have to convert to int.
Why so many globals? Can't these be within the class?
Capitalize the `p` in Python, and expand out "GAE" to "Google App Engine".
updateVariantCost => updateVariantPrice
This is dangerous. Our `is*` functions are used to distinguish the different kinds of proxies, and a non-`list` proxy may have an array value. We will want to fix this soon. I will create an issue.
$user isn't needed anymore, I already fixed the original issue.
Where is the ```\x10`` coming from? is this a \n\r ?
Ah, yes, that makes sense, I somehow overlooked that
Nitpick: Probably should make this a raw string .. turns out '\d' doesnt have a meaning so it works, but still might be a good idea.
Ah, dead end. I was looking at http://www.regular-expressions.info/conditional.html, but it turns out javascript regexps don't support conditionals.
Is turning the proxy off here necessary? Imagine

 1. I launch the app
 2. I connect to my proxy
 3. I quit the app, clicking 'no' that I don't want to stop the proxy
 4. I resume the app at some later time

I kind of expect the proxy to persist after I relaunch...
I would not make ``$sourceType`` an optional argument in the interface. You will always pass it anyway
Mind taking lines 39 - 47 and making a class for them instead of doing this? You can make a new one in /css/slideshow.css
Why did you remove this validation?
We need the icon position for cpanel modules on the left
@ozten - Thanks for the update, I can get the server started but when I enter a big tent supported email into the dialog and am redirected to the BigTent IdP, I see:

```
Invalid email input to service function [:email]string-
```
Is there any danger of overwriting data here? Would sites_signed_in ever exist in both `currentData` and `result`, to where we'd need to add up their values, instead of overwriting `currentData.sites_signed_in`?
we added two new views with this change.
requiredEmail is no longer used, cancelable defaults to true in modules/set_password.  This set cancelable to true as long as requiredEmail was not set.  since requiredEmail is no longer set, depend on the default.

The TOS/PP was only shown on the set password screen for requiredEmail.  Since requiredEmail is no longer used, these options can be ignored.
mark loading needed the sanity check because it selected multiple rows, whereas mark complete selects 1 row to mark complete by its object id, in other words a worker will only ever mark rows it not only owns but has finished.
Does this line belong here?
Definitely, hadn't thought about that.
Also do we not check that the url has changed and ends with '/?s=' ?
no! Ok, that was a bit harsh. So what do we do? Leave it to the documentation is OK for me.
What does a 9 mean? Or where does it come from? Can we comment/document what these magic values are?
@hkenney an empty line to separate here also.
@jeffweiss thanks, changed it
So, this can't stand in Telly.  We need one solution for log output, not two.  While we don't want to change that in 2.7.x, we absolutely need to fix [#13559](http://projects.puppetlabs.com/issues/13559) before Telly ships.
Damn, thanks for the heads-up.  It does exactly that, and my code wouldn't have detected errors.  Fixed now.
Do we remove the added space before the backslash character afterwards?
```
modulepath = $confdir/modules/internal: \
   $confdir/modules/upstream/: \
   $confdir/modules/meta
```
becomes
```
modulepath = $confdir/modules/internal:$confdir/modules/upstream:/...
```
We should try to spec behavior here as how it should work, not that it shouldn't raise an exception. So that means we should specify here that it creates a new pgroup.

In general, any spec that says "should not raise an exception" is almost always a wrong spec.
The timeout can still happen if the write succeeds but the server never responds, or for instance sends a bunch of packets and then idles. So it should be handled here.
Good advice but has a typo.
@jpkrohling yeah, indeed. There is a way to use it as callback as long as you continue returning the value set in the field.
A general suggestion would be to replace 'table' with 'tableName'. This would allow for easier distinction between strings (table names), and instances of table gateways. Especially since table gateways were just called 'db table' in ZF1.
put the bracket inline with the if
bool|null
Yes, sometimes add entities return an incorrect rpp.

Ex :

http://search.twitter.com/search.json?q=blue%20angels&rpp=5&include_entities=true&result_type=mixed

return 7 results.

and 

http://search.twitter.com/search.json?q=blue%20angels&rpp=5&include_entities=FALSE&result_type=RECENT

return 5 results.
Oops. My mistake !
I'd stick to `continue` for readability, but it is purely my opinion :)
@params are in the incorrect order
Could be implemented at a later stage
And another new line after here :)
You can still use overloads to achieve what default parameters would, or you can use DefaultParameterValueAttribute explicitly.
I think you can skip this step. `khan-exercises` is already a subrepo of `stable` mercurial repo.
Okay, last thing, I promise. :-) This is a really minor nitpick, but since `$events` will never be empty, it don't think it makes sense to have a ternary for it. Functionally, I don't see how these two lines of code are any different from the one line you're replacing.

Unless there's a reason for it which I'm not seeing, I'd say revert the two lines above. Then squash your commits and we'll get this merged in finally. :-P Thanks again for your work on this.
This line isn't necessary as it's a repeat of 1.
Use constant.

```
var ENTER_KEY = 13;

...

if ( event.keyCode === ENTER_KEY ) {
```
Thanks for spotting this. I've added a test to the English integration test
as discussed and added a translation for this phrase.

On 4 December 2012 16:37, Alex Tomlins <notifications@github.com> wrote:

> In app/views/calendar/_calendar_metadata.html.erb:
>
> > @@ -1,6 +1,6 @@
> >  <div id="report-a-problem"></div>
> >  <div class="meta-data group">
> >    <div class="inner">
> > -    <p class="modified-date">Last updated: <%= last_updated_date.strftime('%d %B %Y') %></p>
> > +    <p class="modified-date">Diweddarwyd ddiwethaf: <%= l last_updated_date, :format => '%d %B %Y' %></p>
>
> Isn't this going to change Last updated for all calendars. I think you'll
> need a translation string in there.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/alphagov/calendars/pull/21/files#r2308726>.
>
>
why commenting and not removing here?
Replace with: (obj instanceof Pushpop.View) --> true/false
Setting `SITEURL` during local development is a non-starter in my experience, since many URL links are subsequently generated in absolute form. You'll click on a link to a new article and expect to go to http://localhost:8000/2012/11/14/unpublished-article/ and instead get a 404 because you've actually clicked on http://example.com/2012/11/14/unpublished-article/ .
hmmm, maybe a bit more general
```python
try:
    import cPickle as pickle
except:
    import pickle
```
```
use `pass` in function body, fix indentation in docstring (don't use tabs)
Typo: existing.
there is a PEP8 issue here: test_redirect.py:116:1: W391 blank line at end of file
I'm not sure. I just moved the file. Let me look more into it.
2 `  `.
media.style.display = originalStyle;
this should be a parameter ;-)
Spaces around '='
This line might be in there?
Does it work with just the value of the interpolated variable? (I haven't tried it myself, ignore if it does! :) I'm used to the :title => skin.title syntax. (See above, too.)
Failback will be called only after bindToRoute's returned value is called. bindToRoute doesn't trigger failback immediately after route change anymore. Binding to route change within bindToRoute would leave timely unbinding to the mercy of the client code, which would have to always call bindToRoute's return value (and the client code only wants to call it on ajax success). The code that binds to route change is now in loadData function which can unbind on ajax complete.
nit: should this call res.cancel to keep the logic the same?
new detection for journalArticle : detect           : FW.Xpath('//*[@id="docScreenName"]'),
new detection for new itemType         : 'newspaperArticle',
detect           : FW.Xpath('//*[@id="actuScreenName"]'),
I would love to learn doing that. If there is somebody who actually understands what you are talking about :-) then s/he can add such a test.

I guess we will still want to have some more integration-like tests, or am I wrong? (The logic in the rest classes should be minimal and factored out to other, unit-testable classes, so it seems to make more sense to integration-test them. But I have no idea how the bypass server works and what it can/cannot test.)

Are you OK with doing this as a next step or do we need to change it before this can be merged in?
Yes we could do, will patch and add to the pull request :)
Chozo should rescue itself and return the default values if the config file isn't found
This is quite the smell. I'd like someone with a bit more knowledge on Net::HTTP to weigh in here.
These changes means there are a lot more icons to be loaded now.
For client side performance these icons should be incorporated in the css sprite.
You can use `page.ext` here.
Does this only apply for small-bubbles?
Where is the salt?
"should be automatically downcased?"
Can you provide more info on why it would be cool? For me it just seems
like another dependency to the very long list that PuppetDB already has.

I appreciate that (direct and transitive) dependencies are things that are
outside the control of some software projects, but I'm biased towards
keeping them to a minimum where possible or to stick to those provided by
the language writers.

For this reason, I'd appreciate some info on why ehcache is better than
something like core.cache. Thanks.
Should be:

> Edit the Gemfile as shown in "Adding Amazon S3 Support" below (you can do the other steps in that section after your site is first deployed).
Yea, this method will only be useful when it's run against a real app server, fronted by nginx. The response will be the raw picture.
I'll add in some JSDoc to clarify
This can probably be removed for now. I don't think it's an immediate need to be able to run the tests from the admin UI.
This should not be under the getContent component. Might even be worth splitting this out into a new test file.
Might be worth to move this out to an internal _updateGroup function.
Let me put a comment in. The intent is to use a maven profile to automagically switch between all of the various indigo and juno dependencies.
isn't ``!isLast`` enough to express the condition?
Nice tip. I just added it.
Yes, what Marius do for example is that when he hit a CHANGE delta he ask for a Character based diff to highlight the actual difference you see in the screenshot he did.
Probably not :P

Sent from my Windows Phone
________________________________
From: Brendan Forster
Sent: 21/04/2012 6:04 PM
To: Jake Ginnivan
Subject: Re: [DownmarkerWPF] Pulled avalon editor and floating toolbar into a control, cleaning up Do... (#222)

> -                     if (_providers.Any(p => p.View == view)) throw new ArgumentException("View already has a spell check provider connected", "view");
> -
> -                     var provider = new SpellCheckProvider(_spellingService, view);
> -                     _providers.Add(provider);
> -             }
> -
> -             public void DisconnectFromDocumentView(DocumentView view)
> -             {
> -                     var provider = _providers.FirstOrDefault(p => p.View == view);
> -                     if (provider == null) return;
> -
> -                     provider.Disconnect();
> -                     _providers.Remove(provider);
> -             }
> -     }
> +    public class SpellCheckExtension : IDocumentViewExtension

Has anything important changed here? Can only see line ending differences at a glance here.

---
Reply to this email directly or view it on GitHub:
https://github.com/Code52/DownmarkerWPF/pull/222/files#r717154
Yes - we want those to match what we give to sess_set_cookie_params above, since those params get applied to the session cookie.
Да, простите, все верно, не заметил [arrShuffle shuffle]
Программа падает при mutatePercent == 0 и mutatePercent == 100.
ясно, спасибо
mutatuonRange по заданию - процентный показатель, а не количественный.
Жутко неудобный вывод. Постарайтесь просто выводить всю ДНК в одну строку, воспользовавшись переопределением метода description (материал 3 лекции)
if (![a1 isEqualToString:a2])  counter++;  //так же симпатичнее вроде бы :)
I don't think it's in 1.8 though, is it? ActiveMerchant still supports 1.8. We can drop support at some point, but it will need to be a major release.
"This list is called the push attribute list..."
you've forgot to do pass `path` as a first param here
Why this change? Won't it make it match any character?
$this->queryFields = is_array($fields) ? $fields : array($fields);

Imo easier to read.
@Coneko Good point. @jspahrsummers :+1: ?
Ser det ikke i pull requesten. Har du pusha til branchen?
I'm not very happy with this one, it isn't likely to hurt things, but it's not all that save either. I would prefer a path that is fixed somehow, but I have no immediate idea. So let's do this for now.
We may have planned to use this, but figured out a way to do with out it. The variable will be removed.
I'd think you want `change_password_url` here.  And you'll need to set your hostname for each environment if you haven't already.
I wouldn't move this out to the spec helper.  Makes it hard to see what you're actually testing here.
What if we're working against a bare repo?
Maybe should we display the `Info.RepositoryPath` instead.
time_to_index

with a test this would not happen :)
n_ica_components does not exist anymore
copying looks good to me now.
Oh I see, if you only need the csrf token to be available on these views that is OK then.

If you want them everywhere in the project you would need the ``ANON_ALWAYS = True`` and memcached.

It does make sense!
Agree - but my coffeescript / nodejs skills don't stretch that far.

Being able to say ```Robot   = require("hubot").robot()``` seems like the correct syntax, but I could never get that to work.

Could it be something to do with having to include hubot as a dependancy in the package.json?
Diese Änderung ist in meiner Pull-Request "Ja/Nein/Enthaltung" korrekt implementiert, daher empfehle ich den ;)
Jose, thank you for helping! I've skipped my test.
Actually, this can't work...
There are many tupled function and we should create as many tupledX function in Variant... Moreover, tupled calls the apply function from CanBuildXXX also... So this is not really nicer just to go around a warning which is know as not dangerous at all in our case...
No, I did not explecitly reformat this. Maybe WOLips toggled the 'name' and 'defaults' attribute order. If I make changes, I try to do the diff human readable friendly or make style changes in a separate commit.
configuration file varies on distribution and operating system.
This should be

    // ... do something with the color
This is wrong. Devalut vars should be mentionned in a ``.. versionadded:: 2.2`` section
如果使用 -> 这时
<header>Referer</header> 请求头字段包含服务器名中的一个；
=>
           <header>Referer</header>请求头包含某个虚拟主机名；
正则字符串 => 正则表达式
I'm not a huge fan of the name `GetGroupsTokenInformation`, but I'm not sure what a better one would be.
what about

```ruby
@block = block || lambda {|user| true }
```

That would allow you to change `current_user_matches_block?` to

```ruby
def current_user_matches_block?
  @block.call(current_user)
end
```
I've added this line because I've changed the public API of WorkspaceManager, but when I wrote <exclude>**/WorkspaceManager.java</exclude>, it still didn't compile.

I'd like to have a little help on this point.
No need to initialize. The default would be null.
Это и не критично, просто замечание.
Era para simplificar la lógica dentro de la propiedad. Si Gabriel confirma que no hace falta saltar días, lo quito y paso la lógica a VacationsList.
I think that is not necessary specify the third argument.
Ah, I just realised I have to put two hidden fields in the bottom of every form that can be submitted. Then no errors!
Adding create/update/delete tests could be awesome !
Why should LocalStorage not know Settings? I feel exactly the other way around. It should surely get the setting directly from the settings class, instead of giving that through a chain of several method calls. Good encapsulation in object orientation doesn't mean to reduce the number of known/used other classes per se, but to put functionality there where it belongs the most. And the settings class also hasn't to do anything with the graphical settings, if that was your thought. The settings class is on a very low level and can be consumed by everyone basically.
use " quotes (consistency)
key area 2
If the number of keys is generally small, I'd be tempted to use arrays and do the duplicate check manually, particularly if this method is gonna be called a lot. Anyway, such optimisation can be left for later when we tackle performance/profiling.
the change not affect these fixes, only fixes another issue #214
That's probably not going to work in the sharp character is used in a string (or properly escaped), right?
I think that's a spurious feature at best.  I understand it breaks backward
Obviously this statement is incorrect.  Just remove it.
Is inherited. It is not needed for the list but I could add it too
Same as above, ``self.__dict__.update`` is quite cool.
I don't think this return is necessary. Is it?
Yeah let's submit an issue for this.
Shouldn't this be `dashboardactivity_all` instead?
This is really nitpicky, but you can eliminate the ugly cast by doing:

ImmutableMap.<String, Object> of( ... )
because you have already setParameter on queryBuilder before and its better to performance to set parameters before creating query.
Not a webkit error, it just doesn't support that level of granularity so it just drops the decimal.
Confirming that top level categories are ordered by category_position. See http://i.imgur.com/1i2AH.png :)
As stated as comment on your commit, your version wouldn't have run any tests.
Agreed, I wrote myself into a corner, I wasn't going to be able to put them close enough to call it an elf city in <1667 word.

It hurts to be compared to "Indiana Jones 4: This Never Happened".
Kommentare löschen?
Added issue #11 for tracking this.
Similar to the userList getting lost, I was getting errors about messages not having its usual keys here.
Metro mode
`pred` might not return boolean values, so using `false?` as a predicate with `my-some` might not work. You could give the complement of `pred` to `my-some`:
~~~clojure
(not (my-some (complement pred) a-seq))
~~~
Please change these to 12.2.
to escape it or to double it, but we have to allow segment starting by ':'
could you go over these edits with Sherah? I figured she'd find these tursifications interesting
Fixed up in 30d5ba8
There is a racing condition here. You should not check and then save. Use upsert.
But the label should be localized name of the panel.
Noticed now and fixed :)
Use jQuery `.each` to iterate over. You don't need to play with all those indexes. This is not so readable.
HTML element IDs have to be unique over all page. There is no reason to say that uniquely identified element should be child of another niquely identified element. You have already uniquely identified it in the first place. :-)
issubstring() is better here as the string is fixed (and it's not a regexp anyway).
It would be great if this field is declared right after the builder field before the constructor.
This should be indented. You're returning to the dashboard on a failed form.
plus, it would be hard to construct an attack link for this, because you would need an invite key.
are you sure, it works with OnEffectHitTarget?
SetHitDamage() doesn't work with it for me, it works only with OnHit
Sorry, I didn't notice the change in this pull request to use pubchemcompound here... ignore comment on the earlier pull request.
Amb la nova versió d'hyperclient també podràs delegar description a attributes :)
Why "Please" here? It isn't used anywhere else in the document.
no worries, questions are welcome and necessary. It seems that I'm getting it wrong. The diff made me think that the option was added to the function "get_arbiter" but it's not the case in fact. So letting just "hooks" as you did previously is perfectly fine.

Sorry for the unnecessary noise
Certainly it is simple as you said.  But concerning the case when the other libraries depending on Scala 2.10 will be added to libraryDependencies, it maybe that the first versioin is better.
How about createInstanceList?
This should be a:

static vpsized_uint random_num;
But when making a context from the outside you can now refer to it, as in  `this._logger.get_request_context(this._logger.CONTEXT_TYPE_CRAFTER)`. I can make them numbers though, I had to get used to this, but we do it in all kinds of places.
I think. We only scare for the denominator because it's 1/ (2*d[i]*d[j])
It is the Hessian of the function (f_0, f_1,...,f_n), where f_0 is the objective function and the f_k, k\geq1, are the constraints.  In my case the objective is the negative log likelihood plus a linear term (recall my reformulation), and the constraints are linear, so the hessian_wrapper is the matrix [[H, 0], [0, 0]], with H the Hessian of the negative log likelihood.  I added a quick comment to the docstring (comment not yet pushed).
Small comment helping me correlate between the two?
Why fulltitle? Only title.

Translatable string.
I didn't know you could do that, nice.  But I don't think it's a problem to have it public...
`_weibull`
Reverting and checking on the engine code for problems in accepting timestamps.
Doc method doesn't match actual method.
Grr, how dare you undo my little lining up of the comments.  :wink:
That doesn't work here because the dir /var/log/zend/zendserver has a different owner than /var/log/zend does.
Perhaps "messaging.queue.name", rather than just "queue.name"?
No strong feelings...
suggest you move `app = ...` into setUp so that tearDown can always stop it.
58769 is used by the next test. If that goes wrong somehow it affects this one. Not sure why it failed once for me, but I changed this to isolate which test was the problem.
OK, that's a valid point.
Should not call `.isa` here, same reasons as previously explained.
I noticed Nathan has been dropping the _console.dart _browser.dart if the library code is only ment for one or the other.  

So for clarity we want the following:

```
lib/doc_code_merger.dart  <- the import library file
lib/src/doc_code_merger.dart <- the actual source code implementation. 
```
Please use single quotes here.
Method with () means that some action is perfomed during call
B is a current subscription. So no further action is necessary to have B.

None of these are cumulative. They are independent examples to show different intentions.
"_" not "-"
I could comment lines 12-16 and later  a constraint like in line 12 but in the line 18. Because 12-16 lines it's made to avoid redirecting to default_locale every time. What do you thing?
From eyeballing this, I think you need to undef $self->{callid} after using it later. Otherwise if you send a query with a callid to a worker, then a query without one to the same worker, it'll keep returning the same callid? I haven't tested to confirm but that's what looks like would happen :P
This was a test spec written by someone's branch I merged in. I'd like to get it working, but didn't have enough time. Left it here for myself, or someone else, to fix in the future.
I believe you can use TomahawkUtils::defaultPixmap(DefaultSourceAvatar, AvatarInFrame...) so you get the cached version and don't have to re-calculate it each time here
Could all be const'ified :)
The idl says it it a DOMString so it should be fine finally (http://mxr.mozilla.org/mozilla-central/source/dom/sms/interfaces/nsIDOMSmsMessage.idl#49)
Suggestion only :-)
Comment out this part.
it should be `o:code`
Inainte de acolada trebuie sa fie un spatiu: <code>(String XMLPath) {</code>
I totally agree with you :)
Ok, so the one outstanding thing on this pull req is to get Chmouel's vm hosted somewhere more public.
doh! that was very very careless of me, thx.  Fixed.
С английским у тебя всё ок. Но это надо исправить не только из стилистических соображений. В мире C++ принято писать супер-компактный код, а в остальном мире стараются код делать более понятным. Кодировать изменение состояния (префиксный инкремент) внутрь условия - не очень хорошая практика.
Ещё раз повторяю, перехватывать NPE - некорректно. Надо проверять явно на null всё, что может быть null.
Ну так если произошло исключение, то нужно закрыть и выйти, разве нет?
Не по Code Convention
@yuhunglin Can we move this above up :page :godmode:
Need a better solution to this, perhaps using `pack` on the array or using a SHA.
Should be

- homepage
- url
- md5
- version
- head (where available)
Should be `PhCodeSniffer`
Are both of these necessary?
Probably should have added 8.04 to this line.
ResultTuple is still defined in this file.
Ok but we should add a comment to say that this is a test for the backward compatibility layer and have the same test for the official API as well.
now that I see the same example in the plot_faces_decomposition.py I don't see we need this extra example
`ValueError` could potentially be raise by the callable itself and in that case the error message would be confusing to the user. I would rather have a explicit check upstream such as:

```python
if (isinstance(affinity, basestring)
    and affinity not in ["nearest_neighbors", "rbf", "precomputed"]):
    raise ValueError("%s is not a valid affinity. Expected ..." % affinity)
elif not hasattr(affinity, "__call__"):
    raise ValueError(
        "'affinity' is expected to be an an affinity name or a callable."
        " Got: %s" % affinity)
I think it is safe to assume that random Y doesn't have duplicate entries (also the random seed is fixed).
You are right, the entry can not be zero. I made test and comment consistent.
Also we should stop using `auc` in our public API but instead used `roc_auc`. "Area Under Curve" is ambiguous as we have `roc_auc` and `precision_recall_auc` that are both valid yet different scoring metrics for classification.
Team Instance
Can you provide code the code / improvement for this? This does not meet my definition of what sync is, and should not trigger a race condition.
`NativeControlObject::` qualification shouldn't be needed as it's part of the classes parent
This line is repeated
is this method still needed now that you are testing using the abstract method?
The logic of the change seems OK to me; every recursion will subtract 1 from the retries count so it should eventually terminate, and other errors will get raised.

Should there be a short time delay between retries?  e.g. waiting 5 seconds or so, in case it's a transient problem?
TODO ? :)
this one is missing copyright
So the issues is in which order @Id and @DocumentId are discovered. For some reason with our JVMs we always get the @Id first and then the @DocumentId. If it happens the other way around there was a wrong exception thrown!?
Should not be _removing_ entries I think.
用反序列化比较合适
@peterbe as mentioned above, Socorro and Bugzilla disagree on the product name, and we need to provide a link for filing a bug here. We could do this in the view instead and just set a dict called "bug_product_map", although as noted I'd like a more elegant solution just haven't thought of one yet.
If the test fails, the error diff is too long to be useful since it trips the unittest "assertEquals" limit. This turns off the limit.
Not a good variable name. How about `for crashtrend in crastrends:`
Still it's just code that has being moved from request.js but I guess mdn have changed link since it was written.
I guess this is corret one, so I'll update comment to have it there:
https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/encodeURIComponent
you haven't updated `xulWindow` here. To be honest, I'd really prefer this instead of just `xul`.
`xul` and `base` doesn't mean much, except if you played for month, years? with xpcom window classes!
`round` -> `around` ?
While discussing about names, I just realized that `newFrame` doesn't return an iframe but a `browser`.
It may worth saying that instead and may be linking to <xul:browser> doc.

Regarding my previous comment: https://github.com/mozilla/addon-sdk/pull/353/files#r528830
Wouldn't it be interesting to choose a name that reflect that <browser> tag usage?
We aren't providing an high-level API here. The API that is going to use that function will rely on the fact that this returns a <xul:browser>.
I can see value in abstracting/polishing high-level API, but I'm afraid that we won't simplify life of existing low-level developers if we start renaming and abstracting all existing platform terms/details.

That's an open question. I just give you my overall feeling.
I think it is if I understand correctly. If no instance of RouteMatch is returned, that means no specified route matched the request. This is probably more accurate than saying no controller was found (since it's possible to find a route without finding a controller). Actually, this probably needs to use ERROR_ROUTER_NO_MATCH as well
roger, let me do it...
the login class is the only one that needs to reference the url . Your navigateTo method should use a relative link to get to the page you're going to. For instance, you should click the link to open the page:

1. Use a @findby and set the link css path or link text

	@FindBy(css="yourcss")
	private WebElement setupLink;

2. click the object in your navigateTo method. 

	public void navigateTo(){
		try{
                    setupLink.click();
	        }catch(Exception e){
                   System.out.println("setup link is missing");
                }
}
use cc-left here on this entity and remove from definition
You didn't add this path, these URLs wouldn't work.
Is that the current behavior? I thought the current count was "admins plus superusers"?
@branan the plugins will search the appropriate namespaces for the files. The problem we were having was that the plugin should have overwritten `_global_nav...`, but didn't and the app was finding the one earlier in the search path (the stale one). We shouldn't pollute the global area. This will only overwrite something that already exists in the global area.
I tried removing this block of code.


```ruby
                 begin
                         #print_status("Deleting \\#{filename}...")
                         select(nil, nil, nil, 1.0)
                         #This is not really useful but will prevent double \\ on the wire :)
                 if datastore['SHARE'] =~ /.[\\\/]/
                         simple.connect(smbshare)
                         simple.delete("%WINDIR%\\Temp\\msfcommandoutput.txt")
                 else
                         simple.connect(smbshare)
                         simple.delete("%WINDIR%\\Temp\\msfcommandoutput.txt")
                 end
 
                 rescue ::Interrupt
                         raise $!
                 rescue ::Exception
                         #raise $!
                 end
```

It broke execution of the module.
Also, I think `rescale_intensity` could be used to do the renormalization. BTW, it might be worthwhile to add a helper function to get the data type range. I've been calling `dtype_range[array.dtype.type]`, which is a bit verbose.
Since slices are contiguous, can't we just copy out a slice of self.data?
Sorry, I didn't quite get this comment; maybe it's late!
HTML versions of the READMEs?

They are now generated during a release.

On 28 Feb 2012, at 13:55, Joel Tosi wrote:

>> @@ -0,0 +1,37 @@
>> +Logging Example
> 
> Hey Pete - are we no longer doing the HTML versions?  I looked at a few others and that appears to be the case, just wanted to make sure.  
> 
> --- 
> Reply to this email directly or view it on GitHub:
> https://github.com/jbossas/quickstart/pull/114/files#r496096
This documentation is incorrect, since this code does not use dojo at all.  It's also missing param documentation and the required `@exports getJson` attribute.
I agree, which is why my original idea was to simply treat `undefined` or invalid indices as "no image"; but I'm fine with this exception since it's the only good alternative.
Maybe a better way to think about it is to think of EventSource as an implementation detail.  Why do we expect users to actually use it directly?
Should be `czm_inverseModel`.
As we discussed, I don't think that is the case for all ellipsoids and all extents.  I suspect there are pathological cases.  Ask Deron if he originally wrote the code.
I don't think it's clean for this function or `_eliminateHole` to modify its `innerRings` input.
Do we care about idempotency? What if somebody runs the cmdlet twice on the same role?
Use FirstOrDefault
Rule 7 violation
The event name should be all lower case. This is part of the new event system (http://jira.appcelerator.org/browse/TIMOB-10291).
This line also contains a typo where the second `string.replace()` has too many arguments, and will replace it with the wrong value (the first element of the `sdk_dir` path).
Move both of these to a "devDependencies" property. Using the right `npm install` flag, we can the skip those for the deployment.
Maybe this shouldn't be part of the pull request?
These lines appear to be a useful but backwards-incompatible change.
кажется лучше написать != false
Because the info might be wanted for a user who isn't currently logged in. I just remembered that I actually had a use case. 

When a user tries to login and the login fails, this plugin will check if the password for that user might have expired. If so, it will tell the user why his login most likely failed: https://github.com/cosmocode/dokuwiki-plugin-adexpire/blob/master/action.php
Hi Jon,

You have raised several issues which I was not entirely sure the answer to.
As for the bracketing style, thanks for clearing that up; is there a community guide lines document I have missed?

Correct me if I'm wrong:
The aim was to bring meaningful opSlice functionality to a Sequence such that a RandomAccessRange is returned; representing the sequence of numbers within that range.

Example:
For the sequence of all positive integers (excluding zero), a slice of [2 .. 7] would give a range containing the positive integers 3 through 8.

With the sequence's current functionality there is no simple way (that I can see) to apply upper limits.
Therefore a return type of Sequence would be less meaningful than say the return type of take(sequence, upper - lower); where sequence had been lower bounded to 'lower'.

If there is a clearer way to implement this, I am happy to implement it.

On a side note, is this the correct place to discuss these concerns? Or would I have bettter to create a forum thread first?
Thanks for your time, :)

Romm
Sure, but this is the version of `takeNone` which already exists in Phobos, so removing it could break code. It also has the advantage of making it so that the return type is statically checkable as being empty, while `takeNone(R.init)` won't be.
I understand the motivation of this, but unless there's a need for a unified type for all `$` tokens there's no need to add this.
OK, I'll fix.  Sorry if it sounded like I was questioning your judgement -- I'm just leery as a new contributor of rewriting anyone else's code except where strictly necessary for the new functionality.
:) They work for me, yes. At least for weather, transit, bicycle, ...
annotations should still be above the method/class/etc
isn't the version somewhere else defined?