labofsahil/github-event-dataset-2012 huggingface dataset:
Using `QByteArray::fromRawData` here is not safe in general, since it will make the returned QByteArray only valid as long as the MessageIn is still alive. I'd suggest just using the appropriate QByteArray constructor, which will copy the data.
you should be able to just use parseInt:

    parseInt(container.css('top'))
Also, what does calling values_list twice do--does it just combine them?
OH NOOOOES! Shame on my editor :-)
I assume this was just copy/pasted so I will skip over this class.
Actually, I think the "Writing a test" part should go into addaservice instead. Or at least there should be a link there to here, as it is really middleware-related.
I mean ``/products/versions/(versions)/`` :)
Done! Thank you Code Review! I did the logging stuff on like the first day and never looked back if it could be improved. In my next commit I'm leveraging generic_app much more.
Doesn't the above end up doing some saves? Did we reduce any here?
Why not get rid of commitRecovered() completely and just call applyCommit(true) from doCommit()? Contrasts better with the call applyCommit(false) later in the same method.
connetion => connection
I can't find a way to cancel my Pull Request, can't you just close it or something similar? i will send everything from scratch
To be compatible with any future additional servers (newzbin3, newzxxx69, etc), here is a different (better?) regex.
So as to not throw off the group matching, it is non-capturing of the trailing digit(s).

```
RE_NEWZBIN = re.compile(r'(newz)(bin|xxx)(?:[\d]*?)\.[\w]+/browse/post/(\d+)', re.I)
```
What's the benefit of adding this? I understand when it is an object, but here, this has little value.
Can we just use `mixed` then?
``finishView`` is missing here as ``buildViewBottomUp`` is deprecated for the renaming
Mm, right, true.  OK, ignore this set of comments (aside form my general approval here, which is still valid).
Agree. Go with the mock (and more generally, whatever leads to faster tests).
isCover命名感觉怪怪的
If you declare two different functions, /ac and /ac $, the first will only run when you supply no arguments (i.e. getting information/help about a plugin.)
This causes no issues and command helper deals with this.
All default values match their previous defaults in maketx.cpp.
As discussed, I still think we can do away with this argument and the user can just write the two lines to generate the cumulative histogram in their own code if they like.

For oiiotool, those two lines can go into the action.
In the way that, if I were to do:

```
ax.add_feature(cartopy.feature.OCEAN, fc='red')
```

I would get a blue facecolor in some situations and a red facecolor in others (depending on how matplotlib implements alias resolution, and whether that is affected by dictionary ordering). On the other hand:


```
ax.add_feature(cartopy.feature.OCEAN, facecolor='red')
```

Means I would *always* get red oceans.

As I said, this is not something that you need to fix here, just that we need to be aware of it.
isinstance(value, dict) will do the check you're looking for.
@mockee  其实`[self configureView];`放到`viewDidLoad`或者`viewWillAppear`也是比较合理的，这样就不用重写 setter 了，在展现前做渲染其实也是比较符合理解的。
you messages json cant have line breaks in the messages, can it? i think this was one of the bugs we had before...
Pour valider que tu n'as rien perdu au vol si tu nettoie les fichiers js,
l'autocompletion doit fonctionner dans la zone de saisie à gauche et dans
les "reply".

David

On Wed, Oct 17, 2012 at 2:25 PM, Arthur Weber <notifications@github.com>wrote:

> In src/main/webapp/js/tatami-commons.js:
>
> > @@ -631,3 +690,98 @@ $(function() {
> >    });
> >
> >  });
> > +
> > +/*
> > + * Feed the suggestion popup container with the results
> > + */
> > +function feedSuggestions(target, results) {
>
> Ah c'est ce que je pensais xD Beaucoup mieux ta version ;)
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/ippontech/tatami/pull/204/files#r1865887>.
>
>
Another interface?
Just using the File.separator should be fine. IMO, it's no point using a 3rd party library for this.
Yeah, that was the idea :)
Probably not expensive enough to be locking for the odds of multiples being created. https://github.com/jboss-logging/jboss-logmanager/blob/master/src/main/java/org/jboss/logmanager/LogContext.java#L100
This doesn't look like it's doing anything now.
So you trigger the focus handler to refresh the menu, and it'll also remove the focus handler, right? Can we remove a level of indirection here?
"The requirements has changed."

have
Nit: please add a space between `WPA` and `(TKIP)` (same thing for WPA2 and AES)
I'm okay with this, but botio probably won't be :)
mostly just caching them for the swipe manager, since it has its own context
nit: Let's declare var application = e.application; on top of the method and use that
I think `updateNotificationUnread` can be handled entirely in the status bar, but let's merge this a do a follow up if everyone remembers.
I've tested both S2 and otoro, and they both worked pretty well except the first number input. Maybe we can skip or wait a small period of time after typing first number in the next PR.
its below on the evenlistener L53





On Dec 21, 2012, at 12:10 AM, evelynhung <notifications@github.com> wrote:

> In apps/system/js/captive_portal.js:
> 
> > +    } else {
> > +      new MozActivity({
> > +        name: "view",
> > +        data: { type: "url", url: url}
> > +      });
> > +    }
> > +    settings.createLock().set({'wifi.connect_via_settings': false});
> > +  }
> > +
> > +  function handleLoginAbort(id) {
> > +    if (id === eventId) {
> > +			//TODO Close notification
> > +    }
> > +  }
> > +
> > +  function sendUserCancel() {
> But I didn't find anyone calls the function? did I miss it?
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
Oh, I see you wrote in https://bugzilla.mozilla.org/show_bug.cgi?id=766873#c8.  Perfect.  Thanks.
the rationale is that AFAIK we only need to merge the photo and the tel numbers to be sent to dialer, and call log, the other FB data is not needed. We can be more efficient this way. WDYT?
bloody comma after telForm closing
Why do you change this method name?
nit: This makes me wonder if we do not want to completely create this dialog inside the script file if it can be shown only once in the lifetime of the device?
We will always need this unless we have a tile/centered/expand options on settings
typo in "Query"
Added. Required moving the PORT_RANGE into a shared module - currently constants.py. Once we implement ability to dictate the port numbers this will need to be changed.
things are getting a little inconsistent in this file, we have `var pp = ` above and printIndent here
Need to fix the test, not the code. The unit tests are hacks around the code so if stuff is breaking, need to figure out why.
Changed to an empty list instead.
This doesn't make sense. "The login role" can be taken on by anyone at any time, and just refers to load balancing capabilities. I think you're trying to say "The IP of the machine that load balances to this application".
>Added tags support on /f/.
you might pull / merge it back :D
The merge commit also contains that change ;)
https://github.com/childnode/gitosis/commit/fc89aa4e0e31e9511c70ba5de73140589d4e78a2
:) oh yes, that ... I changed that because I was wondering why the iframe didn't give me what I wanted when I was using "href" instead of "src". So I assumed I broke something else... but actually $(...) is fine.
We can get rid of this rect.  I only put it in to make the thumbnail view stop being black-on-black.
If you set up the host to use https you should also change the host port to `443`. Also consider adding a second server block to handle http to https redirection :)
can you add the napatech3gd support also to the configure summary?
Should work according to documentation in `entry.jelly` (and my experience); if you can figure out why it does not, file a bug or fix.
`key` could (theoretically) be an empty string, so you shouldn't access `key+1` here.
This is just an example comment.
I just also noticed that example was wrong since observer takes `subject, topic, data` argument instead :)
Type.prototype -> Class.prototype
I could add `dialog` and `site` methods to the `templates` module. It would simply be moving the code from one file to another.
@jedp - ahHA!  Here is the last offender, [].prototype.indexOf does not exist in IE8.  _.indexOf would work here.
It's useful when we build up the filename from variable components. Echoing the version as it was extracted from the RDF before seeing how it appears in the final filename. I also think we should keen the 'getversion' target and make it a dependency for 'release' and 'build'.
I think this should probably stay 1000.
This should probably be `Puppet::Error` rather than `NoMethodError`, I think.  It is a violation of the semantics of the Puppet DSL, but not actually a failure to call a method.
Same as the note at https://github.com/puppetlabs/puppet/pull/965/files#L3R19.
From what I can see, the only changes to the implementation are the addition of this method and the `NegativeExpectationChain`. Is that correct?
Hallo,
In verband met vakantie ben ik tot 24 september niet in staat mijn e-mail te behandelen.
Neem voor dringende zaken contact op met LICO Innovations via het e-mailadres info@lico.nl.
Met vriendelijke groet,Rene van Lieshout
LICO Innovations
	
		
					
				
					LICO Innovationswww.lico.nl
				
				
					
				
					Bluerailwww.bluerail.nl
``sagecell.util.uuid4()`` doesn't actually exist, you're only defining ``sagecell.util.createElement()``.
Scala compiler fun. Without this, it curiously gets confused about which constructor applies. Previously, we used the 5 arg constructor of which there is exactly one, so it apparently was fine.
Typo, should be `Form` instead of `FOrm`
try to use Array#inject
What's up with this? :s
Why is it necessary to pull immediately after cloning? Also, it should probably be apparent somewhere that what directory you're in when running this matters.
Actually, if you set your background color to an invalid value, the color inversion check fails.  Looking through colorutils.js, which controls the inversion, it assumes that whatever you pass in as a background color value is a valid CSS hex (e.g. "#ffffff").  If you pass "asdf" as a background color, the conversions in colorutils.js break.

But given that, I agree this can be simplified: it's probably best just to require that the url parameter be a valid CSS hex, which is what colorutils.js wants.  This would avoid typos from breaking others' colors, though it would still be possible (as it is already) for a malicious client to send a bogus color value via an injected USER_NEWINFO message and get white-on-white.

I'll update the pull request shortly.
Did you benchmark moving the map into the outer closure? I'm kinda curious about that.
Isn't really clear... What does this one do?
yeah, that's what I was thinking about. I'll remove the extra ":" :)
ratio of exts:types is generally < 1.3:1... so this optimization doesn't really do much in this particular case.
What about this:
```
fluid.find([data.login, cspace.globalSetup.hasPermissions(data.permissions)], function (check) {
    if (that.nonLogin) {
        return true;
    }
    if (!check) {
        // REDIRECT
    }
});
```
I would use ``andWhere`` to be sure not to overwrite some previous condition
seems ok, only wondering if we should switch these to use package: imports? 
#import('package:html5lib/src/constants.dart');
How would this work with serialized/cached instances?
better use the following if you want the repository :

```php
return $this->getContainer()->get( 'ezpublish.api.repository' );
```
rename this to "easybuild-env-vars.sh", and add a comment line on top that mentions for which module + compiler toolkit these env vars were generated
See my comment for the archetype test
Pete,
Can you please clarify this one?
I did a lot of fixes, except the ones that Pete highlighted, please switch to the new branches, it's cleaner this way and it won't have the root pom/readme issues.
I would suggest we just make these simple (no description) links - I don't think that adds much...
The problem here is the two blank lines at the end. It should only be one.

I ran https://github.com/jbalogh/check which is just a wrapper for `pep8.py`
We should remove the extra lines here, as per PEP8.
Pro tip! I didn't know about orm providing a frozen signature for the model. Obviously that's the way to go, so I'll update the code.
I <3 docstrings.
You should make sure that the last byte is always \0 after peekProccing context. If something unexpected (and not zero terminated) ends up in _ccontext the string constructor you pass it to might access out of bounds (and worst case crash the client).
you can use `_email_field.clear()` here which is a bit cleaner.
Please add some hints to help people solve this!
but.. but... 1/(-0.0000000001) which seems pretty close to 1/0 is nowhere near infinity
All the below lines don't need to be in the with statement.
+1 for @jjCavalleri 's solution
simply smart
; не нужна
If we have not been able to distribute/group by groupid - then you probably want a ,MERGE here
the indention is off, can you fix it please? this will make the diff easier to read.
```
check = (cond, msg) -> 
  throw new Error msg unless cond
```
or 

```
check = (cond, msg) -> 
  unless cond
    throw new Error msg
```

I'm slightly morally opposed to the use of boolean operators for control flow.
Do we need this file now? There isn't a Puppetdb face anymore, right?
This should be using a versioned endpoint.
level should be an integer.
I'll go ahead and remove this.
Probably doesn't need a warn. Having memberships is not required for a user/group.
Yea, copying the array to avoid unintended side-effects on the modules array.
This should be removed again.
> How about using CountDownLatch#await instead of the flag + loop? It already supports timeout, is synchronized and everything.

IMO, they are not semantially identical. Calling ``await`` on a ``CountDownLatch`` will *always* block for the specified ``timeout``. I'm asking because I've similar needs, and I find the former solution superior. Anything I am missing?
old habits die hard. zal het aanpassen
These values will be accessed by their index in the arrays later, so if you put one here - you have to put it in `attach()` as well.
Why default 0?
Не буду спорить. Это вопрос интерпретации. Но если в задании четко сказано, что "заменять одну ячейку можно максимум один раз", то про то что символ должен другой - нет. Вообщем выразишь свое отношение в оценке. Я поставил 1 из 2-х и только за не случайность выбора заменяемых элементов.
А если бы здесь был бы реализован алгоритм выбора Х случайных не повторяющихся элементов из DNA, то реализовывать **тот же самый алгоритм**, для выбора значения конкретного элемента, с учетом его текущего значения, в **учебном** проекте я вообще бы считал бессмысленным требовать.  
Но я вот тут что подумал. Если бы d1i1m1o1n стартовал мутацию не с конца DNA, а со случайного элемента, типа вот так:
```
-(void)mutate:(int)X {
   NSUInteger startpos = arc4random() % 100;
   for (int i = 0; i < X; i++) {
        [_DNA replaceObjectAtIndex: i % 100  withObject:[_many objectAtIndex:arc4random() %4]];
    }
}
```
то я вообще бы посчитал этот вариант самым креативным. Все условия выполнены, а код минимальный! И захотел бы поставть 3 из 2-х :)
Решил не менять типа массива на NSNumber. В это классе надо будет прибегать к циклу проверки каждого идекса массива. Видимо там нельзя получить аналогию строки:
[cellMutator containsObject:[NSString stringWithFormat:@"%d", randomIndex]]
Всё правильно, понятия красоты у всех разные. Вполне правильное решение, если не хочется генерировать Exception при неправильном значении, про обработку исключений в задании ничего не сказано.
Here we could use this fllag telling us if the test engine is turned on or not. I.e. if markus_is_test_engine_on && defined? BLAH else nothing. Thoughts?
we lose `this` here, for what it's worth.
Fails also on master, no idea why
End dot missing.
Thanks - don't know why I missed that.
Something is wrong here. Should the first one merely be removed or should it be assigned into a different variable?
Why is this `static`? It looks like an `Activity` is added to this list. Turns out that is a terrible idea because this causes `Context`s to be leaked. See: http://android-developers.blogspot.com/2009/01/avoiding-memory-leaks.html

In a nutshell: Java can't garbage collect any objects that still have outstanding references to them. This means that even though the `Activity`'s `onDestroy` method is called and it's no longer being used, it can't be reclaimed because of the static reference here. It's okay to keep references to `Context`s as long as the object has a lifecycle that is consistent with the referenced object, such as a `Fragment` and an `Activity`.

It's really easy to let something like this slip by so we should watch LogCat and make sure that we aren't doing it anywhere else (I think there are logs anytime it happens).
same here use +=
I would add a note that you trick the stc to use time as frequency here
Against fork point
Is there a reason you use slice here as opposed to params[:page]?
OK, I've come halfway and used a semicolon here as the clauses are connected, but the second isn't strictly the _cause_ of the first.
...Which doesn't sound as a good idea anyway because AFAIU we would like to get rid of plugin.

Maybe creating a component for managing wikis (i.e., provide a subset of what wikimanger does) could be an interesting alternative.
These ones slipped through. I am doing a search on the code an will remove all of them.
do a match instead
И тут в обоих функциях та же хрень с fowrard decl и const ref. А еще строчка пустая перед signals :3
Es cierto, igual ese atributo en este momento solo describe el campo, no
tiene ningún efecto. Crees que debería sacarlo o hacer alguna otra cosa?

On Thu, Oct 4, 2012 at 9:49 AM, Alpha <notifications@github.com> wrote:

> In source/CommonJobs/CommonJobs.Domain/Employee.cs:
>
> > @@ -18,6 +18,13 @@ public Employee()
> >              AttachmentsBySlot = new List<SlotWithAttachment>();
> >          }
> >
> > +        [Display(Name = "Usuario")]
> > +        public string UserName { get; set; }
> > +
> > +        [Display(Name = "E-mail corporativo")]
> > +        [DataType(DataType.EmailAddress)]
>
> Pero ese no tiene atributo DataType.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/CommonJobs/CommonJobs/pull/193/files#r1761207>.
>
>
Doesn't look like a new entry is needed here, see 3 lines down, ::name-must-be-unique-within-org.  it's the same regex.  i suppose you can argue that the match is coincidental and may not always be the case.
In a way, yes. The `order_choice` scope requires a dev to understand what options it can take. Also, I picked up on some duplication using `where("created_at > ?", date)` with 3 of the case statements. I thought it would be better to re-write to a more generic scope that takes a date and makes the comparison.
is it ok to import modules using relative paths like this?
Ruby methods actually often don't an explicit return statement. The method returns the value of the last statement executed by the method.  Leaving off the return statement would give you the same result.  Even so, nice use of string interpolation :)
``$sendCredentials`` would be better
You mean non-const then? It's not universal reference if it's const if I understand correctly.
The issue here isn't for the models though, its the JSON the user sent to the controllers missing a required key. The controller validation method should catch that.
I just realised this is a configuration bean.  Does this mean we lose the ability for people to provide custom CH implementations?
-1

What should be fixed is getConfigurationAsXmlString(), not show individual configuration options (it would be quite a considerable amount of methods...).

Also, if you're going down the path of showing individual configuration options, show them all, or show none.
No we should not restrict it.  Just use the default sync repl timeout if the cache is async.
Yap, remove em.
Grunt creates a dev dependencies to itself, on it´s own, when using grunt to create a grunt plugin.
I´ll just mirrored it.
btw, why using a static property here ? Other filters don't do it.
So, the deal here is that the header will, in reality be NUL terminated. But you should probably take that as a coincidence, because the lines of actual diff data with definitely not be NUL terminated (because you are just being given a range of characters in a loaded file).
Good point.  I agree with both of these comments, but I think in the end it probably makes sense to assign to shape so that we'll get an exception if we break something in the future where the dimensions in fact don't match.
Does this only produce a pdf? (there was no png or svg added in the commit if not)
I worry about the complexity here.  Apart from the difficulty in understanding what is going on, there is the fact that tight_layout is called on every draw().
Unfortunately inplace won't work here because we can remove pixels from the original image, or further differencing will fail.
Note, I don't have a problem with the use of *args and **kwargs in the saving() method below, just that given that setup() is acting as the terminus of the call tree should probably at least warn that there were extra args.  Case-in-point, misspelled args to savefig() never get noticed.
I know its not a public interface, but a docstring would be nice.
Your indentation is so weird.
Thanks. I'd used `adapté` later on, but just didn't notice it here.
Most of this was translated with an idea, but this bit I just went for a more direct translation.
marking it as generated in thsi case would be nice
You could use a guard to prevent this additional indention-level
should be "opengeo"
.. and those 4 deleted are added twise too. So it was wrong cll about delete some strings. Anyway, diff gives sometimes strange results for humans ;-). I really don't like git, used to work on svn, so it's lerning curve ;-).

I'll update this file in a while.
fixed in luvit pull request
why ``-v`` ?
Should the return type not be "array|Value\Messaging\ErrorMessage|null" instead of "array"?
Should the return type not be "FeedRenderer" instead of "FeedStrategy"?
Should the type for parameter ``$groupPath`` not be ``string|null``?
Sorry I may be wrong. I couldn't fined corresponding Jira issue (maybe searching with wrong keywords) and also Html docs generated from source were not updated properly.
As long as Titanium.Event.source is implemented than there is abug and I opened a jira bug for it - https://jira.appcelerator.org/browse/TIMOB-10316.
the last line still multiplies by 2 rather than scaleX
Removing the leading slash so this becomes…

```jinja
{{ asset('bundles/google/js/gdash-1.0.js') }}
```
DRY. 
Vorschlag: Wie wär es mit einer Abstrakten Klasse "PageableServerResource" die von ServerResource erbt. Die Konkreten Resourcen erben dann nicht mehr direkt von ServerResource sondern von dieser Abstrakten Klasse ("PageableServerResource"). Dann hätten wir den Code nur einmal.
Won't that make all the other groups uneven ?
i think we should default false and mention that setting it true will require the phpcr sonata admin bundle to be in the project. or is it possible to load the service definition if sonata is not available?
Also, maybe call this `overall_pay_rate` instead, since (analogous to the `median_pay_rate` method), having a mean_pay_rate that's simply `mean(pay_rates)` also makes sense.
The point is that your regex matched also `account/confirmation/toke/`. I do not know why you have that first `?` in there. This makes `n` optional. ;-) And then you also have `/` optional. So you get `account/confirmation/toke`, and `n` is captured into `confirmation_token` and then follows `/`. :-)

You didn't made `confirmation_token` optional. So it had to be matched. So `n` went there. And the rest was left out because it was optional. So if you want that `confirmation_token` is optional, you have to make it optional. So the best is simply to go like you want it to go. Everything until `account/confirmation/token/` is mandatory. And then the rest is optional. And how is the rest constructed? From `confirmation_token` and `/`. And this is it.

Your interpretation of regex is a bit strange. You should do not talk about adding, but about matching. Imagine like regex overlaying over the string. So you overlay `account/confirmation/token/`. And then you have one `()` section which is optional. This is made with `?` at the end. Which means zero on one match. So if will match preceding part zero or one times. And preceding thing is made from `\w+` which is captures into `confirmation_token`. And `/`. The same would be to write:

```
r'^account/confirmation/token/((?P<confirmation_token>\w+)/)?$'
```

Only then the `()` would also capture everything inside. Because we are not interested in this group, but only the part without `/`. We use special `?:` at the beginning to tell that this group should not capture anything but it is there just to combine things together. (So that last `?` can work on everything inside or nothing.)
Probably `horoscope` word is unnecessary here?

`piplmesh.panels.horoscope.providers.get_provider` tells enough that it is provider for horoscope.
Hm. Maybe we should have first two sections, basic info and additional info. Then we would have sections for each login method: ours, Facebook, Google, ... where people could link accounts if not already linked. Or configure username/password if not already by going the a separate view. Once registered on our site, there would be link to password changing form instead.
Yes, I ran the code and it worked, not even a error in chrome console (?), that's why I didn't notice the typo, so now I just removed it
Why? Why you cannot simply iterate over anything returned by `fetch_data`?
`buildFragment` only uses its second argument to get a `document`, so there's no reason for it not to just accept one directly, making array creation here unnecessary.
No one ever likes `unless;else`.  I agree in simple cases that only involve the order of the conditions, `if` is preferable.  But here that first line would read

```ruby
if !(item.is_a?(Array) && item.is_a?(Hash))
  #...
else
```

which I strongly feel is harder to parse.

I used to joke that Matz should change the `else` keyword to `otherwise`
Последняя проверка зависит только от текущих условий, поэтому так делать нельзя.
could we give this a more descriptive name?
filename is called `v6` but classname is `V4`
Don't use System.out.println for logging. Use a java.util.logging.Logger if you really need to print out information of some sort.
you should use None as a default value here
I see no reason to guarantee any order of invocation of listeners. Unless it is a requirement of some set of specifications, this looks a lot like overkill and/or encouraging an anti-pattern.
I prefer the string because it is lazy loaded, and can prevent circular imports.

Also to keep track of the app where it is coming from I always per-append the app name it even if it is from the same app.
Sets hosted services. (I think it is good to keep the description consistent with the name of the variable/methods. ).
:+1:  Patches are coming in an hour. I have to leave now.
How would you propose that work?
I didn't know the method (until you just mentioned it) but it takes an NSDict, which we don't have here?
Do you mean `- (void)enumerateObjectsWithOptions:(NSEnumerationOptions)opts usingBlock:(void (^)(id obj, NSUInteger idx, BOOL *stop))block`

Using `NSEnumerationConcurrent` may well be faster there. We should probably use it everywhere tbh!
Let's rename this namespace to `cartodb.core` since the name `client` conflicts with clj-http.client.
I don't think it's necessary. It follows the same pattern, and it's better if the examples are not too long.
Nice!  Hadn't seen that one before :)
OK, then we will have only header search bar. And since we will have header, I would recommend that click on the logo would take the user "home", because it look funny if we have another button to return user back home. And also css should be edited, because css class from google is overrun by our class.
Typo: *warrned*.
This whole test is being ignored...
What is the plan to address this TODO?
I will clarify the dox here and in Shear.h and shear.py since real-space position angle vs. complex phase seems like a natural point that users might wonder about.
Feels confusing, as there is already a concept (in AWS at least) of an AMI owner. Which is very different from this tag.
chiffres?
the declaration misses "void" here:

mongo_servers* mongo_parse_init(void)
{
Oops, this is my rebase miss.
Are you sure that shouldn't be "errmsg" (with an extra "g") ?
Heh, copy and paste error - it had "bool or long" before - will fix.
Which of the next 3 statements should raise a ValueError? All of them? Only the last?

If only the last, then this test will pass incorrectly pass if an earlier statement raises ValueError instead.
Not your fault but probably best to save the type as a var above the if to avoid all these long if statements :)
I'm on the fence - how many ValidationExceptions exist in the global namespace? Also, is it not better to extend one of the child Exceptions like InvalidArgumentException for semantics?
Better/simpler check is `if ( empty( $_POST['ef_insert_title'] ) )`
Yeah, ```has_one``` and ```has_many``` are almost identical.  In ActiveRecord the difference is just that has_one slightly optimizes via LIMIT 1 and doesn't return a collection.

We probably don't really need a ```has_one``` method unless a use case arises.  But this method here I think should be renamed to ```belongs_to```  just to be consistent with ActiveRecord conventions (since has_many is typically associated with ActiveRecord.)
На null надо проверять явно, а не исключения ждать.
Что за replaceFirst? Что за Message? В задании ведь формат чётко описан.
Зачем этот new String()? Это лишний объект.
Здесь произошло исключение -- FileReader не закрыт.
А если там есть строка "STOP"?
Extracting the Ray code out into its own class was necessary for making it easier to do UI testing.
Done: https://jira.media.berkeley.edu/jira/browse/CLC-699
You may need to reverse this so that users can override the default curl options.
return self::$CURL_OPTS + $this->curl_opts;
s/storae/storage/
> qid is fine to me, as it's the same name as in the file format...

I would also prefer 'query_id': I understand query_id, but not qid.
you should not need to hold with pylab. It's on by default
Capitalize a the beginning and full stop.
Ask @NinjaZidane, I just wanted to let you know. Oh, sorry for the mail @NinjaZidane :p
I think that there might be a few issues with just using IsLatestVersion, I think there is also an IsVeryLatestVersion or something as well.  Not sure if the intent is to not get those or not.
Object doesn't have an isExpression method. Is there another way?
Not available with Maven compilation.
Merci pour l'astuce :)
Done
This is from new line.
This `if` is useless, no?
silly "nit" but this is easier to maintain if written as:

```
attributes = [
    'one',
    'two',
    'three',
]
```

Then the indentation doesn't depend on the length of the variable name. And diffs look a lot more plain if we need to remove or add things to that list.
Again, I don't think you need this.
Document this module!
Why don't you do this check first and return kSpdyReachableViaWWAN?
le risque c'est que du coup le : ou le ; se retrouve à la ligne suivante, la règle exige non pas un espace comme ceci " " mais un espace insécable " " qui correspond au &nbsp; mais le problème c'est qu'il n'est pas correctement affiché dans les anciennes version de IE (6 et 7)

Exemple : 
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec et mattis massa. Quisque leo felisa, fermentum a condimentum at, tristique :
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec et mattis massa. Quisque leo felisa, fermentum a condimentum at, tristique :
*beancounter* should be a local variable.
Thanks for the reply. Can you please replicate the empty-password login?
The behavior I replicated in Bug #10 was the lack of HTTP Redirect in case of unknoww
Use the decorator require_POST:

```
from django.views.decorators.http import require_POST

@require_POST
# view
```
But this "if title" prevents it from displaying, right?
pep8 (80 characters wide column), otherwise LGTM, +1
I'd check for a truthy value here instead of `is not None` to avoid the empty string passing the check.
else => print_error

Is always nice to warn the user when things don't work :)
There are better established ways of doing this via Railgun actually that allow us to grab all domains, and all DCs for each one of those domains. 
If you look at https://github.com/rapid7/metasploit-framework/commit/89e554de2b95a6de0d2471edfd1194796c7057d6#L0R151
you'll see how Railgun can be used to grab all domains the machine knows about and all the DCs it knows for that domain. You can then check each DC for the Groups.xml file. This will give you much more pervasive coverage
Right, but that was in another PR.
I wonder how this could happen. I 100% sure I tested this... strange.
We try to follow the [numpy docstring standard](https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt) so you don't need the back ticks surrounding the argument.
I've updated the DEVELOPMENT.txt file.  Have a look and let me know what I can still add:

https://github.com/scikits-image/scikits-image/pull/255
Could you line wrap this? (around 72 columns would be great)
Looks like this was a nasty bug.

We should make it more clear that we are writing to `collection._boundingVolume`.  Why not clone to it here:
```
var boundingVolume = updateBoundingVolumes(this, context, frameState);
```
and then assign it to the command.  Otherwise, I had to look through a lot of code to see why this change was made.
@kristiancalhoun can you please review the doc for this pull request?
Why did we flip the order of parameters?  Where they wrong?
I'm not sure we want to have console.log anywhere in our code.
OK.  Sounds like you have it under control.
Did this bug make its way into beta or stable?  If it was Canary only, we can remove it.
I tweaked this a bit.  Do you want to mention any other lower-level public features?  Did we had useful abstractions for web workers?
This change is probably wrong.
In vertical layout the vertical offset is calculated based on `boundingValue` and horizontalOffset based on `bounds`. In horizontal layout both the offsets are calculated based on `bounds`. Can we move all offset calculations to `bounds`
If the handler for the event can't be called, shouldn't we just return?  Do we even want to bubble it up?
Yeah... I think we should bring this up in architecture. In my opinion, having a platform-specific parameter is such an egregious misfeature that we should try to eliminate them rather than adding documentation support for them.

If iOS adds a platform-specific parameter, and then Mobile Web adds a different platform-specific parameter, we have essentially split into two methods with different signatures.
This can return pendingAdds as well. Is that safe?
call requestLayout() here too
what do you think about --log-level (or --loglevel) instead of the mixed case?  Or are we getting --logLevel as a standard from somewhere else?
Why not put the code in the if block to a separate method in KrollProxy.py so we don't need to copy it over here?
There should be only one `start` call if there is only one `stop` call (or an explicit stop via `asyncTest`.  As it is now, I have no idea which one of these two is supposed to run.
The separation of classes still is not as it should be. You see that, because you need stuff like the static_cast when calling the installation function.

The base class should define the common API, which the UI needs to install packages. This includes storing the state of the installation, i.e. which repositories to add and which packages to install. As this is the same for all backends, the class should contain the functions and variables to store this state. These aren't abstract functions anymore, but that's fine. It's common to mix some common concrete functions and some abstract functions, which need different special implementation in the same class.

It's also important to get the names right. This is essential for later readability of the code. Go with concise names, which clearly indicate what the function is doing in the context of its class.

Finally it's important to document the API. This will make it clear, how it's intended to be used, and it will also expose issues with the API, because when you notice something is hard to document, the API is probably not good enough yet.

So the base class would be something like this:



```c++
class PackageBackend
{
  public:
    /**
       Add repository required to install the selected packages. This will be added to the system as installation and update repository.
    */
    void addRepository( const QUrl &url );
    /**
      Add package to be installed.
    */
    void addPackage( const QString &packageName );

    /**
      Resolve dependencies of package selection.
      
      Returns an error object, indicating the result of the resolution.
    */
    OCI::DepencencyError resolveDependencies();
    
    /**
      Install application. This adds repositories and packages to the system, which were set by the addPackages() and addRepositories() functions.
   */
    void install() = 0;

  private:
    QList<QString> m_packages;
    QList<QUrl> m_repositories;
}
```

The fake class then just has to implement the install function and show a dialog with the repositories and packages to be installed.

The real backend class would actually perform the installation.

Later we also will need to add some progress and status API and improve the error handling.
I am concerned that a race may occur between this test and other tests that add reviews which will make assuming that if the first review is different then the one we want has been deleted. This is especially pertinent because this assertion is a negative assertion.

I think we need a way to check that the review is not on the page in any position, ie matching the `h3>b` title of the review against those on the page.

Just checking the first page (most recent 10) should be sufficient.
Aesthetic: I usually like to have
#endif /* WITH_ZEITGEIST */

in case of long conditional blocks.
I'm being lazy, not you.
I'm sorry for not explaining my logic sufficiently. Regarding,

> popFront clearly describes a function, from my viewpoint, it is a clear cut case that it should be @property

there have been several arguments made in past discussions about the evils of Methods-as-Properties. Specifically, when a (badly designed) API uses a noun for a function then it may be used in a not-obviously wrong way. For example, `Class.noun(value)` vs `auto result = Class.noun(value)`. While a valid argument, it doesn't hold for APIs that aren't abusing/using ambiguous verbs/nouns for their functions/fields. (Besides `auto a = appender[].popFront;` doesn't compile.) Which is a very long way to say that `popFront` doesn't require `()` for it to be obvious to the reader that it's a function and therefore there is no reason why it shouldn't be `@property` in my mind. Which gets me to my next point.

As for properties in general, there are a lot of different view points on what exactly properties are and should be. I'd recommend reading's Bertrand Meyer article on the subject for a different take on the matter (http://www.eiffel.com/general/column/2005/Sept_October.html). Also, in D, `@property`'s original purpose was to supplement Methods-as-Properties in order to disambiguate a corner case involving fields/methods returning zero-argument delegates, with the side effect of allowing Java/C# style properties. But, again, original intent/roadmaps has little to do with use today or use tomorrow.
`opEquals` for structs can still return int though. Shouldn't we try to fix that? (And even introduce a compiler warning/errror maybe).

The docs:

`If structs declare an opEquals member function, it should follow the following form:`

```d
struct S {
  int opEquals(ref const S s) { ... }
}
```

It works with either int/bool.
I think we've reached an impasse here.
`gen` is a very common and rather obvious abbreviation, and I'd personally be against using the word `generate`, because `gen` is plenty clear and much shorter, but `rangeToCRC32` works just fine. It's just not what I would have picked.
Good insight. I tested removing the version statements and the forward reference error disappeared. For now I've changed the associated comment to:

    // @@@BUG@@@ structs declared inside version statements are subject to 
    // forward reference issues, so we must define DirHandle and DirEntry here
Missing comma at end of line...
wouldn't it be better/an option running the enhance via a "regular" hook?
maybe we should use something other than ${project.version} for the openengsb dependencies, cause this is the reason for the problem mentioned here: https://groups.google.com/forum/?fromgroups=#!topic/openengsb-dev/KGWTFWONxwo ?
there should be never any deprecation without providing a possible replacement in the comments

/**
* @deprecation blabla
*/

. If it is simply no longer required pls also state this in the comment.
What's the intended/planned storage for the local credentials? (I'm thinking specifically about a way to possibly use this via webstart)
Sounds good. Could you create an issue?
why mocking the abstract class ?
Makes sense. I also didn't really like the foo/bar. Will change it with something more descriptive.
Not surprised; removed TODO comment.
I don't see a test which asserts that Playtones is executed multiple times when necessary.
I'm sure there's a less variable for this size ...
I'm not sure why this changed. Previously, we only did gsub if the subkey wasn't empty.
What do you think about changing this line to:

while not valid_date?(date)
or
while !valid_date(date)
?
ok, this is a really really minor thing, but it just caught my eye this time through - normally I wouldn't put a blank line here, since the "try" and the "except" go together logically, it feels odd to me to separate them visually. But I don't really care, feel free to leave it as is.
Css for new default template file
used json_encode instead of get_object_vars, because get_object_vars only turns the first level of the object into an array. Now the entire object results are turned into an array.
There are ways to demand an implementation id.  I think MDS does it.  Doing
that can be a ticket to be done later.
I think you meant (StringUtils.hasText((String) replyToStack) here.
Never mind, I got it.
Aren't you mixing runtime and schema attributes here?
Yeah, either that or

the ~~final~~ result is to throw the exception to the caller after the __final__ retry is ~~exhausted~~ attempted.
Please use caps in the package name - HTTP is an abbreviation.
Eliminate dead code if not in use.
Can you align $tracker with the start of the first argument? (ie the first quote character)
@bp323 This should probably be an H3
Из какой папки берётся CSS?
not sure we need this method as this can be done in the template.
I'll fix the self to this.

How would you write the default options then? Believe me, I'd much rather write:  
`$this->setRegion($region or $this->_region or self::$_defaultRegion);`, but  PHP doesn't have logical or...
You should be able to require this to be an Integer -- DWR will handle the conversion for you from a string in javascript.
Sadly, no, because JSON doesn't allow JavaScript comments.  But I think the description fields are a good place to put that kind of information.
I don't think we need interactive things yet, I'm all for reserving that for future pull requests.  We also need to add overlay support first.  However, I think being able to just visualize the satellites in Cesium Viewer is the minimum first step before we can pull anything in.  I would recommend we not merge this request until the matching Cesium pull request is also ready, this way they both go in at the same time and are in definite agreement.
is that just "if (! search['multiple'])" ?
Yes, done!
better to remove these commented lines
Actually all map sizes above 7x1 px working correctly, non square maps too. They just dont fit in the level select menu.
Can you change this to an English text? In another pull request/branch we should change this to translated texts, but we want an English backend when editing a dutch page. This is not possible at the moment.
add  <   />
can be removed due to the load_and_authorize_resource
This condition is evaluating as true even when there is no cookie, meaning that we never try any of the options which follow.
I'd call this `syncable`, but then I love brevity. It's fine if you don't change it though.
is -> are

App properties can also be set at runtime by the application in Javascript
will return them in the in that order
=>
will return them in that order
looks good. we will have one norms per experiment
I'll will go with some - more awesome.
These are the diffs of the removal
My editor is set to automatically remove empty spaces from lines

These are the removal of white spaces not additions :)
It's harsher to suggest not accepting the offer!
hardwaremodule are 2 words