labofsahil/github-event-dataset-2012 huggingface dataset:
1) Tbh I don't feel comfortable with the apophatic way of checking things here. Would be lovely if the condition could be formulated as "tpe is XXX || tpe is YYY".

2) Also while we're at this fragment of code, maybe a comment would be in order here. When I took a look at this bug today morning, I couldn't immediately understand why exactly would one want to insert apply in adapt.
question: is this supposed to minimize until it converges to local minimum, or it minimizes only a few steps?
`args_cnc`, again, could be used.
And you can use pythonic integers in some examples, SymPy calls `sympify`.
But there are double variants, when you work with Rational.
If you treat integer as pythonic, and divide one by another, then you obtain or pythonic int or pythonic float.
But if you want to obtain sympy result, then you must serve as input (for python interpreter) sympy type.

See "Using SymPy as a calculator" in the [tutorial](http://docs.sympy.org/0.7.1/tutorial.html#introduction) for details.
possible rewrite:

```
# in case Add(y, x), whose cache is equal to Add(x, y), is already hashed
# we clear the cache to make sure that this test is independent of cache.
# Note: with the Manually constructed Add(y, x, z) this was not necessary.
```
I kind of dislike both.
In the finger docstring, a, b and x have the same first 4 values. I pushed a commit to my boolequals that provides a workaround for the count_ops problem. It would be better to have `count_ops` working for Booleans...I wonder if a recent change changed their behavior.
I disagree.  We should be able to organize the docstrings however seems best.  I also like to use "Example" when there is only one example.

Having a whitelist would be a major headache.  Any time someone edits a docstring header, they have to remember to update the whitelist.
Do we actually want to sort by `Float` objects. This starts to look a bit like http://en.wikipedia.org/wiki/Inner-platform_effect
that is fine, but when `options$fig.align` is `'default'`, you should not put `swfhtml` in `<div>`
Well, that's expected behavior by default, I think. Is there a real need to mention it?
Because the docs describe for example, 'n==1#one book|n>1#many books'.  I just think it would be better to be more explicit about the fact that the expression is separate from a message by "#", not by "|"---The pipe is instead separating the pairs.
checkboxe typo
zafinlabs in gwtp ?
ok reverted the test and fixed xmlns issue

-------- Original Message --------
Subject: Re: [karaf] demos poms; karaf plugin (#6)
From: Andreas Pieber <notifications@github.com>
To: apache/karaf <karaf@noreply.github.com>
Cc: Andrei Pozolotin <Andrei.Pozolotin@gmail.com>
Date: Fri 17 Aug 2012 08:09:14 AM CDT
>
> In
> tooling/karaf-maven-plugin/src/test/java/org/apache/karaf/tooling/features/GenerateDescriptorMojoTest.java:
>
> > @@ -45,4 +51,25 @@ public void testReadXml() throws JAXBException, SAXException, ParserConfiguratio
> >          assert s.indexOf("repository") > -1;
> >          assert s.indexOf("http://karaf.apache.org/xmlns/features/v1.0.0") > -1;
> >      }
> > +    
> > +
> > +    /** FIXME xmlns 1.1.0 validation */
> > +    @Ignore
>
> can you pls add the issue ID to the fixme then? (that we don't loose
> the connection here)
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/karaf/pull/6/files#r1402446>.
>
Sorry, already handle by https://github.com/fabpot/Sismo/blob/master/src/Sismo/Notifier/MailNotifier.php
Are we sure that this method is unwanted? It seems to be that if we delete it, then removing and re-adding a pair may result in a mis-sync, since the checkpoint will have survived past the pair removal.
So, what are these bad at that "normal" geo is good at?  Why wouldn't everyone always use geohaystack?
Looks like this change causes the Aurora logo to disappear when the flag is not active. For example, compare...

* [https://developer.mozilla.org/en-US/ Prod]
* [https://developer.allizom.org/en-US/ Stage]
Shouldn't `language-rels` always be done after migration, by default? Or, at least when `--all` is specified?

How do you picture this being used in real migration, or the incremental migration cronjobs? A bit confused here, since it *does* seem do migrate something.
pep8 whitespace on blank line
should use " on outer and ' on inner parts
Should be prefixed with phpbb_ I guess?
Why the check for contents === false? That should never happen.
I get some sort of error with changing all is_enabled to enabled:

PDOException: SQLSTATE[42000]: Syntax error or access violation: 1072 Key column
 'is_enabled' doesn't exist in table

C:\wamp\www\phpbb3-fork\tests\test_framework\phpbb_database_test_connection_mana
ger.php:327
C:\wamp\www\phpbb3-fork\tests\test_framework\phpbb_database_test_connection_mana
ger.php:155
C:\wamp\www\phpbb3-fork\tests\test_framework\phpbb_database_test_case.php:90
C:\wamp\www\phpbb3-fork\tests\notification\notification.php:63
C:\wamp\bin\php\php5.3.10\phpunit:46

Line 62 is:
60-	protected function setUp()
61-	{
62-		parent::setUp();
Parentheses around `!$is_greater_ie7` are unnecessary.
I would think this should be qualified with whether the event is dispatched before or after the user is deleted.

Why is post_username passed to the event?
Get the list of enabled drivers from avatar manager. Eliminate instantiation of disabled drivers. Eliminate is_enabled check here.
"array of users" or "array of user IDs"
those should probably have the `phpbb_` prefix like `$phpbb_functional_url`
Some comment, limit 1 and free result comments here.
Undefined should not be quoted in my estimation.
@rphillips thoughts on this?
I need to think a bit about the privs, but I would be happy to merge this in now. One change: maybe move this to `insertLabeledRecordP` instead, and just `mappend` the supplied priv and dbPriv
Hmm... I guess I'm suggesting something like `Y.namespace('Env.Mojito.Handlebars')` then.  The "Env.Handlebars" seems very generic, and somebody else might be using that as well, causing us to trip each other up.
Мне твой вариант не нравится по одной простой причине - на крон возлагается ещё одна работа. Логика работы твоего варианта - крон  берет данные из одной таблицы, при необходимости обновляет таблицу баннеров, таблицу из которой он брал данные(потому что нужно число показов за предыдущие дни) и таблицу статистики за день. То есть он делает 3 вещи. В моем варианте - всего : Обновление баннеров и того же самого числа показов за предыдущие дни
This could be replace with `return t.IsValueType && !t.IsEnum && (t.IsPrimitive || t == typeof(decimal) );` which is a lot more compact. Of course your'd have to add in the generic stuff as well
Missing XML comments of interface and members
names is confusing especially due to congflicts with other things in pymvpa with the same name
Yes. Is there a way in javascript to 'name' different indexes in an array? 

So instead of 
getFields()[5] 
I could just have
getFields()['indexForBlah"]

I don't like putting these as constants in the RaxaEmr_Controller_Vars because the fields/indexes are likely to change as we find we need more.
handler to controller? (this comment applies throughout all files)
As the order is not important, it is implicit that the order of dimensions in ```weights``` must match that of  ```array```? That is, if weights is multidimensional, the order of its dimensions must match the corresponding subset of dimensions present in ```array```. If this is the case the docstring should say so.
why not?   i want to retry generally, but if its a crazy number of retries something else is wrong and I should break.   100 catches the crazy.   1000 would also catch the crazy.   10 might not catch the crazy
cwd not used?
Suggestion: use of the plural for a variable that has many elements
By passing username in to the implementation constructor
Now that I have taken a closer look at the CompMap and IsoVector API's - doesn't the IsoVector api provide a bunch of methods that can manage this sort of computation for you? - Should this take an IsoVector and a qty instead of a CompMap?  I'm not saying we necessarily should - just wondering what you think.
Because you may need that information. Doesn't hurt to pass it.
I think this existing behavior, i.e. `deleted_objects` get reset when `save` is called.
@marchof Could you please remind me - why we need random id for relocation of classes?
enjoy the vacation!
You've got two single quotes here...
I would think that an example query that uses the learned coefficients to predict some results would be helpful and important for: 1. verifying the coefficients are correct (since it is hard to judge whether ARRAY[-3.579, -5.99, 0.636, 0.451, 0.0581, 0.112] is correct without any reference), predicting the results on the training set and comparing it with the ground truth would probably be enough; 2. providing reference for users who want to use the coefficients to perform predictions.

Some ad hoc queries (prob. using madlib.array_dot, madlib.logistic) should be enough.
We shouldn't hard-code it, should we?
Do we support "any type" in the future once we get more generic transition state?
Did you mean to write "line" here? ;)
Oui c'est bien ça. Je ne sais pas pourquoi j'avais l'ancien nom
Wouldn't `lrange(0,-1)` be simpler?
Here's the other side of having the verification stuff in the Data Object classes: It obscures what exactly is being checked, making it too hard for non-Ruby programmers to see if the right things are being tested.
Yes, but as I've pointed out there, you've actually misused the singleton pattern (as in it should've been a function factory).
{Array} [arg]
This sort of "fuzzy" matching leaves a dry taste in my mouth. Is there any way it can be made more explicit that "location" has wiki stuff?
Weird.  Fixed in the real code, but it's not showing up in this partial diff.
Is this needed if you're not using dojo.query?
I think you could just use StringUtils.htmlEscape() here
The following block:

```
    if (tokenType === HTMLUtils.ATTR_NAME || tokenType === HTMLUtils.ATTR_VALUE) {
        if (tagInfo.position.offset >= 0) {
            if (tokenType === HTMLUtils.ATTR_NAME) {
                query = tagInfo.attr.name.slice(0, tagInfo.position.offset);
            } else {
                query = tagInfo.attr.value.slice(0, tagInfo.position.offset);
            }
        } else if (tokenType === HTMLUtils.ATTR_VALUE) {
            // We get negative offset for a quoted attribute value with some leading whitespaces
            // as in <a rel= "rtl" where the cursor is just to the right of the "=".
            // So just set the queryStr to an empty string.
            query = "";
        }
    }
```
can be rearranged to be more efficient and easier to read:
```
    if (tokenType === HTMLUtils.ATTR_NAME) {
        if (tagInfo.position.offset >= 0) {
            query = tagInfo.attr.name.slice(0, tagInfo.position.offset);
        }
    } else if (tokenType === HTMLUtils.ATTR_VALUE) {
        if (tagInfo.position.offset >= 0) {
            query = tagInfo.attr.value.slice(0, tagInfo.position.offset);
        } else {
            // We get negative offset for a quoted attribute value with some leading whitespaces
            // as in <a rel= "rtl" where the cursor is just to the right of the "=".
            // So just set the queryStr to an empty string.
            query = "";
        }
    }
```
Checking for "file types not supported by Live Preview" (i.e. doc.extension !== "css") should be move to a function.
Oh, you need to replace Tab with spaces in your new line.
Since it's a string I think `line.substr()` would be more canonical.
ニャッスニャッス
id might not be an integer
To get configuration to work better. I expected configuration to override in this order: defaults -> config -> commandline. Before this change you couldn't enable/disable a server from the config file. The defaults got removed because store_true and store_false don't work very intuitively when they have a true/false default set as well. Merging the commandline options at the end would result in a true/false always being set in the parser object as wall as the cfg module.
see

@property
    def report_context(self):
        context = super(ADMAdminInterface, self).report_context
        context.update(
            detailed_description=mark_safe(self.detailed_description),
            adm_item = dict(
                type=self.adm_item_type,
                form=self.form_class.__name__
            )
        )
        return context

On Sep 13, 2012, at 3:09 PM, Cory Zue wrote:

> In corehq/apps/adm/models.py:
> 
> > +FORM_KEY_TYPES = (('user_id', 'form.meta.user_id'))
> > +CASE_KEY_TYPES = (('case_type', 'type'), ('project', 'domain'), ('user_id', 'user_id'))
> > +KEY_TYPE_OPTIONS = [('user_id', "User"), ("case_type", "Case Type")]
> > +
> > +REPORT_SECTION_OPTIONS = [("supervisor", "Supervisor Report")]
> > +
> > +COLUMN_TYPES = ('form', 'case')
> > +
> > +class ADMEditableItemMixin(InterfaceEditableItemMixin):
> > +
> > +    def __getattr__(self, item):
> > +        return super(ADMEditableItemMixin, self).__getattribute__(item)
> > +
> > +    @property
> > +    def editable_item_button(self):
> > +        return """<a href="#updateADMItemModal"
> 
> should this be mark_safe?
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
Is this needed ? Or can we use CSS classes instead of an if/else ?
Personally I like the overriding of __eq__ and __ne__, even if the costs is to ovveride it in loss_curve, loss_ratio_curve and conditional_losses too.
I'm glad you removed this. It looks like it wouldn't have functioned as expected, as both lines were indented, but only one was within the else. If you make it a habit of using curly braces around blocks like this, you won't have to think about it.
-> Antoine Gourlay
I think it's not necessary. We are not in win$. If the user click to execute an empty script and nothing appears it seems a normal behaviour ? no ?
Aw we said "Trop d'information tue l'information" ;-)
Too much logic. If errorcode is 0, return. Otherwise, `rugged_exception_raise`. That's it.
We will probably not even need it, because we will be doing REST requests. And those will be authenticated with OAuth one day. :-) (And I was lead to believe that then you do not have CSRF problems. I am not sure about that though.)
Ok so the first part is done, when user connects to our portal, language sets based on browser settings. Now i just have to add option "language" to user object, which will be by default the same as browser settings language.
I have to say, I try to just blindly follow here. `syntax_upper` and `syntax_lower` errors do not go to `default_error_messages` because these errors do not get displayed to end user?
Need to call this function, and add a semi-colon.
Missin' semicolon.
If a spec is no longer valid due to the code changes, please remove it instead of using xit
This whole block ended up being small enough to incorporate into ```os_data()```. There's no need to factor it out if it's not used elsewhere.
ace_type is really perm, yeah? that might be more clear
Oops, It should be:

    if (ctx == NULL || ctx->modules == NULL) {
'is' is correct here since the subject of it is 'the maximum'.
似乎没可能
这个tried写在peer_data里面，更好一点，在init_peer函数里面初始化。
Numeron sijaan suosittelen käyttämään merkkijonoja ```"eventKey"``` ja ```"name"```
This method has changed it's return value now, if we care about that. Perhaps just have it return `true` after everything?
Please use CollectorPathFilter available in LocalStreamService. You can move it to FileUtil
Move the code in merge stream data consistency to an abstract class and use it.
Can we avoid this path listing and use the same listing done for orderly creation above? We might have to pass the lists outOfOrderDirs and notCreatedMinutePaths to the method as parameters and get them populated instead of method returning it. What do you think?
这里任意项都是相关知识，所以不用这么写。直接写

```
- 语义化
- balabala
```
Seeing this line reminds me that this line could be removed as we control all Javascript and libraries used.
s/packaged/officially released/
Will fix!  Was a firefox vs chrome thing

On Mon, Aug 6, 2012 at 1:57 PM, Jason E. Rist
<reply@reply.github.com>
wrote:
>> @@ -15,6 +15,22 @@
>>          %label
>>            = _("View")
>>          %select
>> +        %span.view_tipsy.details_icon_grey
>
> This icon appears to be poorly aligned vertically.  Can you fix it? (hint: float right)
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/Katello/katello/pull/434/files#r1313979
We may need to set the val to '', if the settings.value === i18n.unlimited.  I think what may happen here is that if the value is initially 'unlimited', user clicks to change it and unchecks the checkbox, the input will be populated w/ unlimited string, which might be confusing to the user.
`delete this.emitter`
Actually, do you think this test should pass? It seems weird to allow comparisons between Quantity and Unit, but it may be confusing to the user...
Sorry this was a typo, I fixed it afterwards (but not soon enough!) If you go to the github pull request page you should see the updated version of the comments.
I didn't know Boost has such a thing, thanks. Do you suggest to use and include suffix.hpp or just do this?

<pre>
#define STRINGIZE(X) DO_STRINGIZE(X)
#define DO_STRINGIZE(X) #X
</pre>

I think the later is nicer, as we don't need the rest of suffix.hpp and it perhaps confuses the windres rc-compiler.
Now you copy the string instead of using it as-is for no clear benefit?
agreed. I will add this tonight
Naming it EnrichmentFacility means that the old one (which offers swus) needs to be renamed to be reinstated. That's no problem. Just a note.
You can use delegate to multiple methods if they have the same options. eg:

```ruby
delegate :group, :discussion, :to => :motion
```
I think we should also handle this in the previous clause where there is already a selection.
I think you'd be better off installing to `tempfile()` so it's guaranteed to be unique
N.B. `join` here is `os.path.join`, which isn't the right thing for joining URL components. It will co-incidentally work on *nix platforms, but presumably do the wrong thing on Windows.
I assume the reason you specify a whitelist is the skip IPython.zmq and/or IPython.parallel.  This should not be done.  Also, I would recommend against using pip to install pyzmq, as easy_install will get binary eggs for appropriate platforms.
This line should probably be moved up by the other attribute definitions for `Cell`.
Good idea. I've fixed this in cb93822
This is an unnecessary step in such a tight loop.

```
cb((const git_oid *)current, data);
```
Haha, this is indeed stuff of nightmares, but I can't come up with a cleaner way to handle this repetition.

Regardless, I think that all the code block that get passed to the macro should be in brackets, otherwise the code gets ambiguous and hard to read.

~~~~~c
GIT_DIFF_COITERATE(
    onto, from, o, f,
    { delta = diff_delta__dup(o) },
    { delta = diff_delta__dup(f) },
    { delta = diff_delta__merge_like_cgit(o, f) },
    {
        if ((error = !delta ? -1 : git_vector_insert(&onto_new, delta)) < 0)
            break;
    }
);
~~~~~~
Not a good thing. The XDR paths should be queried at the same time we query the user's home dir for `~/.gitconfig`. They don't need their own API calls.
>80 lines, boo whore

also, you misspelled because.
That would be better but it's a lot of code to do the feature detection. Feature detection would also have to be tested to ensure it isn't too slow during page initialization.
An `L10n` comment describing that this is a tagline for Firefox Android would be useful.
This img still needs an `attr=""` as well.
I <3 these comments.
Shouldn't this be named self.installed_manifests?
Heh.  Clever.  I have this itchy feeling like this might bite us at some point.  But I think it's ok.  Nicely generic, but we may find we need something more specific some day.  I'm cool with this though.
As you changed the fields returned by the query, you will also need to change the dict creation at the end of the function, or fields returned by the service will make no sense. 

Please edit the documentation (both in this file and in middleware.rst) according to those changes too.
Why are we checking for `openform` here? Does it imply edit mode? If so, should it go into `PlominoDocument` also?
The class name for this changed, didn't it?

Why did it change?
this should use ``{@inheritDoc}``
Do we need to check the children at all ? (what if the root form is an empty collection)
This should be kept. If you explicitly pass `null` as a `$path`, it should be chnaged to `/`.
`$htmlPattern` needs to be public (this is how the constraints works) but method visibility shouldn't be public unless the methods are part of the API. `getNonDelimitedPattern` is an internal method and should be private (and moved after all public methods according to the CS).
it should be ``if (!$this->has($key) && ...) {``
"Does this apply to all predefined PHP classes/interfaces? Or only the ones extended/implemented."
I meant, does this also apply to core classes that are used inline? like in a method.

For example.
```
        $formatter = new \IntlDateFormatter(
            \Locale::getDefault(),
            $format,
            \IntlDateFormatter::NONE,
            'UTC',
            \IntlDateFormatter::GREGORIAN,
            $pattern
        );
```
Uses this, but its not mentioned inside the CS.
So if the documentation must be updated I will open an issue in the documentation section ;)
`$request->isMethod('GET')`
We should recommend to use Scrooge instead https://github.com/twitter/scrooge
We will deprecate the custom thrift compiler in favor of Scrooge.
Indent this little guy 2 spaces, since he's inside the p block
http://dustinbrewer.com/your-last-css-attribute-doesnt-need-a-semicolon/
This method looks like it queries, delivers, and marks as delivered. Thoughts on separating the concerns into different methods?

```ruby
def self.deliver_automatic_reminders
  due_for_reminder.each do |invitation|
    UserMailer.reminder(invitation, invitation.sender).deliver
    invitation.reminded!
  end
end

def self.due_for_reminder
  where('sender_id IS NOT NULL').
    where('created_at <= ?', 5.days.ago).
    where('vote_id IS NULL').
    where('reminded_at IS NULL')
end

def reminded!
  self.reminded_at = Time.now.utc
  save!
end
```
dependant -> dependent
Parens around params.
I'm now of the opinion that wms iterrogate is out of scope.
I have no problems scanning the xml from a *getcapabilities* request.
The above text could be replaced with very simple instructions for this.
Will do that next...
I see, thanks. Added to *test_wms_retrieve* in coming commit.
writing it to stdout is probably a safe bet
Yeah, it's just the same i suggest in pull request. Btw, It's not my script, I burrowed it :-) Surely i will fix all this nasty things
`str(e)` not required
fixed in [97366c0](https://github.com/mockee/BookBot/commit/97366c004b0b5b6a513ec2a0d59ef622be45a2c1) and [d95e030](https://github.com/mockee/BookBot/commit/d95e030eba132e8b772114e8b17b93137d56b537)
@everbird 我搜了搜，不太明白要怎么改。明天中午咨询你吧
Can you add a `capifony_puts_ok` at the end?
Answering my own question: ok, so firstName() and lastName() are not used in `static::$formats`, so it's not a big deal.
"Mettre à jour" ?? Vu que tu as mis l'infinitif ailleurs
Do these tests really have to be run under the full profile?
My line of thought was that the security check at the top of the method has already passed so the code has the permission to lookup that particular name. The security action is there to not require any further checks (with the caller's access control context) but rather assume that the lookup is allowed (which it is).

Now I'm not 100% sure that this is the correct approach in case of continuation so please correct me, if I got that wrong.
Is this absolutely necessary to achieve the goals of this patch? Moving the plugin version back in just one of the many poms in the AS has a bad smell.
Is it possible for a system message to launch an app that is not an activity, but does not need to have its size set?  

I've heard that the homescreen launch is going to stop being an activity. If that happens, then this test will need to come after the homescreen-related code that is below it, I suspect.
Nit: 2-space indentation please.
Same here, prefix the labels with some word reflecting it belongs to the Japanese IME
It works for me in testing, but I suspect that is because the images are already loaded, so it is fast.  To be safe, you really need to put everything after the img.src = src line into img.onload = function() { ... }
Storage.get? yes, it's using localStorage which is sync.
we have a task to replace it all with indexeddb so it'll by async.
anyway the init() and requestUserLocation() are not called after another- the init is on ev.me load, and the requestUserLocation is only when a user taps a location-aware app.
looks like your PR is based on an obsolete version of the Settings app: this part should **not** be deleted.
A few comments here:

1) I'd put the parseInt() up on line 286: parseInt(target.dataset.index)

2) on line 295, consider using the file type instead of the extension

3) Also note that we'll eventually be able to get rid of the storageIndex stuff, I think.

4) Whenever you call URL.createObjectURL(), you are also supposed to to call URL.revokeObjectURL() when done with it.  I don't fully understand when it is safe to do this (we should ask someone like Jonas Sicking when he's back from vacation).  Maybe try adding an onload event handler to the audio tag, and once the song has started loading, call revokeObjectURL().
`delete`
I’m not a native English speaker but precisely, that’s what I’d have called a “switch” instead of a “toggle”.

Nit: indentation is incorrect.
my text was eaten jejeje in the previous comment

div id="search" data-currentPage=true
Can be fixed in a followup.
To clarify this will likely totally break when CSP is enabled.
jinjao and jingo after django?
I purely don't understand why this check is needed. Is any realistic example?
Only inverseTransform on things like Paths, which act as if they do not have a matrix
Really good note.
That is probably a rare, but possible, situation - it could occur if we failed during the first broker handoff attempt, I think, but retried.
Not sure whether we should have the nofollow or not - leave that up to you.
Yep, I'll correct this
Would prefer this to have a dash instead of space. Also there should be a prio for passed tests. Either the same or passed-test
Ok, misconception on my part. Carry on ;)
Do you want to also add a spec for this job?
Yeah, I wanted to keep it simple for this pull request and avoid breaking something accidentally by missing a `setAttribute` somewhere.  So Tag and Mixin nodes just inherit the functions from Attrs.  I wanted to keep it DRY without risk.
Not sure how to explain it better. It means at the end of the function results will be results[offset:]
It does not return or raise anything and so I dont list those.
You don't send anything but a form.
This is a strange error check. Why are we check this versus a dictionary type? Generally in a dynamically typed language, if you're doing a type comparison, you're doing it wrong. Can't you just do:

    if not r_op.json:
I had to write that suggestion here. Sorry.
are you sure this will work on the **entire prototype chain**? configureBeforeFilters is located on `Controller.prototype`.

see 06eb37dc173971beb569740c0e459f2f56803e34 for my solution on stuff like this
trailing blank line
Hi Rolando,
*cc.Config.deviceType* has been changed to *cc.Config.plaform* in the latest *cocos2d-js-tests*.
So why you changed it back to *deviceType*?
should be indented one level more
refactored code, creating methods to be overridden
Sounds good. But i am not sure where i would put the defines then ? Can you help me out on this?
Hum, it actually also works like without the space! ;)
Passes jshint?
Maybe include a URL that tells you more about the drupal dblog (possibly on the drupal site!)
I think there's a strong case for using event fields to control what gets sent to cloudwatch, but it probably should not be the only way.  i'll meditate on this some more & follow up in the jira ticket.
worst case, if you're unsure how to refactor this based on my comments, I can merge this and do it myself - either way :)
Rather, 'master' should be 'v%VERSION%' literally, so that when the docs are generated, they're generated for the version intended.
este foi o único teste que não passou aqui.. lembrando que o peter tá fazendo algumas alterações no chartdata, então vamos precisar voltar nesse teste na semana que vem.
No, it's an object that was stored in a slot. I've added a note.
Right, I have not realized that :) In that case sure feel free to keep as is.
Sorry to bug you again about that, but I still consider this as being hard to follow.

    let sandboxOptions = {
      principal:
        'principal' in options ? options.principal : systemPrincipal,
      sandboxPrototype:
        'prototype' in options ? options.prototype : null,
      wantXrays:
        'wantXrays' in options ? options.wantXrays : true,
      sandboxName:
        options.name,
    };
    if ('sandbox' in options)
      sandboxOptions.sameGroupAs = options.sandbox;

I thought you would have ended up with something better from what you suggested yesterday.
But it is still unecessary hard to read. Mixing input and sandbox options is still done.
The mapping is still hard to follow. For example it is not obvious by reading code that options.name is mandatory.
Then there is some weirdness introduced by this pattern,
`prototype` default is `null` (`prototype: null,`), but then you default it to `{}` (`sandboxPrototype: options.prototype || {}`)
You are returning an URL here, whereas `toFilename` should only return file path!
When the xpi is packed (which is the default behavior, except when `unpack: true` is set in package.json),
we can't get file path for any file from data folder. As these file are read directly from the xpi file,
and their final URL will be jar:something.
Nice. Yes, we have to publish data pretty early so we don't have nay blind spots. Good call.
__whimboo__ commented
> Where does PopupNotifications come from?

It comes from [Firefox](https://developer.mozilla.org/en/JavaScript_code_modules/PopupNotifications.jsm):
> Note: This code module is imported by Firefox chrome windows, so you don't have to do it yourself in most extensions.
$arr_uids = array();
これと
Don't call the constructor, we might be using it for something else as well.
Can you rename the variable to `validateBehavior`, just to clarify :)
This ends up checking that we can round trip the string, not the time. 

I'll fix them up the tests as part of the review.
That fails if `build_dir` has spaces or other metacharacters in it.  We should be robust against that.
Yep.  The idea is that we want to find an expectation that matches the name and can still be called more times.  If we can't find one that can still be called more times, then we fall back on one that matches the name but is fully "consumed".

FWIW, this same pattern is in several places in this class, and I think it's a bit convoluted, too.  I'm planning to refactor, but to do that separate from the bug fix.
We certainly could.  We could make them different, too.  I just copied and pasted to make it easy on myself :).  @dchelimsky -- got a preference?
@weierophinney `isset()` works with `0` :)
You right, but the output is consistent with the filename generated using the normalize parameter.
I mean:
$ doc2rst.php -d foo.bar -n
Will generate foo.bar.rst and if the document include a foo.baz file will convert to foo.baz.rst.
So I think the behaviour is consistent.
Ah, you're right, i will close my PR, thank you very much.
Please always use mixedCase for Archetypes field names!
@CHH to be exact, it is not because it already have an id. It is because the object itself is already in the identity map. If you clone your object and persist it, it will create a new entry, even if your PHP object has an id already.
This does **not** follow PSR-2 as you are not using camelCase for the variable you introduced :)
This simply fixes the wrong calculation of the changesets and yes, it would be calculated again, if there where no changes (which is still better than it was before). Having the other array would work the same way, set in computeChangeSet, reset in commit and the other places ...
Please add a proper error message for the exception
I thought that would be the simplest case. You set up your importer with our gem, run it and you would instantly get an output. If we use a file by default, then I imagine the user would not find the file instantly. He would have to look for it (or read the documentation or the code).
You might not want to use white explicitly for the folks using lighter color schemes in their shell.
two spaces before #
You can just use this format for throwing errors, rather than using the `new Error` syntax:

```javascript
throw 'Unknown or unsupported transform functions: ' + unknownFunctions.join(', '));
````
It's fine to leave 'em, but I'm pretty sure you don't need the `\` because you're already inside the function call parens. My syntax checker seems to thing so anyway. No biggie though. Just a comment.
This looks like it's missing a . or # or so!
Looks like there's an extra space here, between the def and get
This entire method is now run in the instance in lieu of starting a process.  I will add some data length checking soon.
I think you mean `String` instead of `Integer`
Not sure but if I remember right we have some way of iterating over dictionaries, so you can do `data-each="CHOICES as key, value"`.
minor style nit: We always use `{}` blocks, even if there's only one statement. So instead of:

    if (USEB)
        vertscale -= B + 5;

we prefer to say:

    if (USEB) {
        vertscale -= B + 5;
    }
Actually, I shouldn't have made the method non-static, because it hides the intent - all the loggers share the same locale - so I'll make it static, so final won't make sense really
Not that it makes a difference, but `i++` is clearer.
But why? If you're giving it a default, why force people to set it?
Then modify pretty_print_dict ;)
New line. Also, use jQuery?
Ok, but this won't break as far as we are reusing 'eea' domain in other packages?
fixed by accepting your PR
Ideally we drop the ownCloud reference. I'm don't at all have a problem giving credit, but if this is something you'd want; we could add a CREDITS file, or something similar :)
when this option is enabled, the Remeber me checkbox is a bit lost :-)
![Remember me](http://vrac.pwet.fr/admin3_login.png)
избавляемся от копипаста!
Sad. I hate it when my brilliant revelations are obvious to other people :)
Sorry about that... I caught it a bit late.

Anyway, the issue with the external resource is that it is an external resource when it can be easily internalized. A bit less flexible, but is one less complexity to go wrong.
Yo should use a if here
Reference to cloud sdk is odd, and I suspect this may be some old/internal naming ?
src/openslide-vendor-leica.c:84:11: warning: unused variable 'view' [-Wunused-variable]
We really need separate wire format docs for reporting. What the data structure looks like, what encodings are supported, how are timestamps interpreted, etc. 

Basically, the same kind of stuff that's in the catalog wire format documentation.
`on-shutdown`, maybe?
`null` instead of `false`
There's already a checkPassword function in oae-authentication/lib/api.js.
oops, that got in through a merge conflict, I'll clean it up
Is it necessary to remove it completely? I know we're not writing code we don't need yet but deleting it is just throwing away time and money imo.
I'm fine with removing the REST Endpoint since it's not used.
Do you need ``"T" -> Type, "TRAIT" -> Trait``?
тут щось не так з порядком аргументів. див https://github.com/stfalcon/BlogBundle/pull/9/files#r1565085
I would count null as technically valid in a data store, so I test for undefined explicitly.

I usually test for truthiness with whatever calls get('key'), but I don't wanna assume null isn't a valid value.

On 2012-11-02, at 4:25 PM, Jen Fong-Adwent <notifications@github.com> wrote:

> In public/js/app.js:
> 
> >          })
> > +      }
> > +    })
> > +  }
> > +
> > +  function get(key, fallback) {
> > +    if (dataCache[key] !== undefined) {
> can you use a truthy here or is it b/c you count null as valid?
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
Thinking about it, shouldn't be better if Geocoded takes the responsability to set CountryName if not set while CountryCode is set? I mean, this should be a standard result based on some kind of standard country code, doesn't it?
Почему предполагаемый размер массива равен mutatePercen, а не numberIndexesToReplace?
2goodman116: Здесь действительно косяк, перепутал местами числитель и знаменатель.
I don't know the galaxy code base, so I don't know what is better.  You are going to have to suggest something if you want a change here.  I do not think putting it in the build.properties is interesting, but I don't want to add another configuration variable / command line parameter either.
are you sure it's not if (!found) return?
ohhh right, duh, yeah I think that's good
I don't see how this is actually unrolling the recursion. What am I missing?
No need to pass in ```params``` here, since it's available in all of the controller.
`k2pdfopt_mupdf_reflow(page->doc->xref, page->page, page->doc->context, dc->zoom, dc->gamma, 0, width, height);`
should be 
`k2pdfopt_mupdf_reflow(page->doc->xref, page->page, page->doc->context, dc->zoom, dc->gamma, 0.0, width, height);` 
or bb_width will be 0. I don't know why. Shouldn't an integer instant be cast automatically to a double when calling function? I only test on emulator on Ubuntu 12.04 AMD64.
`resource` will always be a `resourceInfoType_model`, won’t it? So this test should be superfluous.
space before equal operator.
Why did you need to remove explicit return types? It avoided to run the type inferencer (and thus made the compilation time faster).
Thus, you should not use unlink on a class property but setting it to null. unlink will delete the property from the class (leading to a strict error on next access as it would be undefined)
Why blocking here?
NULL check should be added
Ну, после всего цикла. А то так и будет нулем.
No estoy seguro de entender tu pregunta.

    if (lastSentDate.Date >= date)
        date = date.AddDays(1);

> Si el último envío corresponde al día de hoy, programa el próximo envío para mañana. 
> En cambio si el último envío es anterior a hoy, programa el envío para hoy mismo.
¿Seguro que no querés catchear errores en un attributo? ¿Por qué es eso?
Creo que esta fecha es 11/7, ¿no?
Yes, a singleton class would mean there is just one global collection. Example of a singleton type collector:

```
module Collection
  @@items = []
  def self.add_to_collection(item)
    @@items << item
  end

  def self.all_items
    @@items
  end
end

Collection.add_to_collection :the_item
Collection.all_items
=> [:the_item]
```
Code Style : missing blank line
Per @ajdavis we should probably note that this is the maximum number of idle connections that will be left open.
Hmm, not necessarily related to this PR, but I think you need to get the cache members here and not the JGroups view members.
Sanne, shoul be better to put this comment on the java doc of this class? Or it's ok to be here?

I think all you comments in this class is about understanding the way I'm using this. (and maybe there is a better way to do this work)
It is intention to hardcode version here?
committees.aspx.cs isn't evening using FindElection and is building the criteria itself so it shouldn't affect anything there.
missing space before `right`
please use this style for if:

if( test ) {
Always a space between `if` and `(`
Since functional review was good I made the change and squashed it into your commit.
please always have a whitespace between if and ( and for and ( etc, like

 ``` if (foo)```
actually this will cause build errors. I will remove that - please fix your code to work w/o -lpthread here
This </div> should not be here! :(
Was this really supposed to be changed to `fhtml`? That won't actually select anything.
I'll change that to form_name. I initially had that as form_id, but realised, in cases where you import from one deployment to another, with form_ids that correspond to completely different forms, you'll end up having a report tagged under the wrong form. Having the name ensures you're tagging a report to its intended form(form_names are unique).

No, you shouldn't have any reports with blank form_ids. They should either default to 1 i.e the default form, or have another existing form_id. Did you get this from the download or are you referring to DB content?
Sorry *big teeth smile*... I tried to do that but the layout is being "destroyed" when I try to do that. Any advice?
Should I do a table just for this message like it appears in the viewtopic_body.html ?
The user puts the location to look for the backup files here (Input Directory)
Should the return type not be ``\ArrayIterator``?
how come this one is different from the scaleY of other files?
would this work?
long sleepTime = System.currentTimeMillis() - lastRun + MAX_AGE / 2;
Thread.sleep(sleepTime);
Lol I had that. I'll put it back.
To be consistent with the text above, the comment should be "if r > 0, spherical ROI of radius r"
True... f9d106d
I knew that I should write a piece of code like that, but this implementation does not look very elegant to me. What if someone downloaded a tarball? I'm not opposed to this solution, just not sure how good will it work in corner cases. Do you have any similar examples from other projects?
I know. But why to have two-level dispatch? I don't think it is necessary. You can always simply have common prefix or something. `post_new`, `post_update`, `post_delete`.
Please list first CSS, then JS.
OK. Then do concatenation of first and last name.
I would not have min length here, so that error is displayed only once (for first field), if password is too short (and everything else passes).
I don't really like this. It destroys the clean separation of frontend and backend code! We could use [signals](http://readthedocs.org/docs/mongoengine-odm/en/latest/guide/signals.html)? But maybe signals on Mongoengine are too low level (what happens if we store the object, but Tastypie or higher levels crash). So one way would be to define our own signal dispatch at this point. And then in frontend code connect to it.

[Django signals](http://docs.djangoproject.com/en/dev/topics/signals/) are used exactly for this purpose, to decouple code so that you can have code doing one thing somewhere and another thing somewhere else. It is a very simple concept, you just define a point in code where you say "at this point I will call everybody who wants to be called" and then you attach other functions there, which are then called.
`$(document.createElement('div'));` -> `$('<div/>')`
I would use for alt only "google" (this is what is written on the logo).
s/reults/results/
@Sharpie: In regards to the cmake build process, I'm not using `std_cmake_args` here because when I did, the quotes around the prefix flag caused problems. Specifically, it substituted in `-DCMAKE_INSTALL_PREFIX='/Users/<username>/.homebrew/lapack/3.4.1/'`, which then caused the build process to think this was a relative path, because the first character wasn't a `/`, it was a `'`.  I'm not sure if this is something specific to this package or global, but I think it bears reporting.
Filename should not contain any '/'. That's what I did there. Replaced any '/' with '_'.
output=open('..../.../..,''w') will create an IOError
This does not change the folder structure.
Before merging... what do I have to do to enable the tests? (also consider that the webroot is currently wrong)
I'll test it again on latest TC rev. I think the spell itself is working fine but the client update after casting is missing.
same for all those
Please add a new line at the end of the file
Here, you need:
public function roll($successfulAttack = FALSE)
You probably could have just added font-size to this parent element to specify it in pixels. Since "em's" are relative, everything else should have automatically fallen into place. I'm not going to block on this though, as I'd like to revisit the dimensions of various aspects of this AlertView at some point in the future to ensure consistency with native components.
Ok, ich denke das ist keine gute Idee, das Problem ist die unterschiedliche Formatierungen zB Console/ANSI ....etc
does this needs a `notifies :restart, "service[jenkins]"` or something, or is this one of the benefits of using WAR?
... and all sub-directories recursively beneath it.
So I should just write "()" instead of ['127.0.0.1']?
this is also try in other places in the code IRRC : all the default values like "tcp://127.0.0.1:5555" should be constant in a single place, so when changed it's changed in a single spot
Capital T
You would have to ask some designer I guess. :) This comes from the tables originally. The box-shadows are not related. I agree that `0.05` would suffice though.
Does `req.requestbody.content` always exist? Also on line 342.
I agree. Too late for translations, but at least for the english one.
Bug: textExportDirFin
> Maybe you’re right. Whenever I see “CF” instead of “NS”, I think it’s old
Carbon crap that isn’t going to work much longer, but that’s not true, is
it?

Nah, from what I can tell, most Cocoa just calls `CF` functions. Look at
stack traces in a lot of our crash logs - Cocoa calls we've made end up
calling Carbon apis which are the ones to crash (The ones doing the brunt
of the work)

On 18 June 2012 14:17, Rob McBroom <
reply@reply.github.com
> wrote:

> > @@ -12,12 +12,34 @@
> >
> >  @implementation QSDownloads
> >  - (id)resolveProxyObject:(id)proxy {
> > -    NSString *downloads = [[@"~/Downloads"
> stringByExpandingTildeInPath] stringByResolvingSymlinksInPath];
> > -    NSFileManager *manager = [[NSFileManager alloc] init];
> > -
> > +    // Try and get the user's downloads folder setting (set in Safari)
> > +    NSData *downloadsData = (NSData
> *)CFPreferencesCopyValue((CFStringRef) @"DownloadFolder",
> > +
>                    (CFStringRef) @"com.apple.internetconfigpriv",
>
> > Can NSUserDefaults get prefs from other apps (not just its own)?
>
> Yeah.
>
>    NSUserDefaults *prefs = [[NSUserDefaults alloc] init];
>    NSDictionary *pdict = [prefs persistentDomainForName:@
> "com.apple.Safari"];
>
> > Anyways, the NSUserDefaults API would probably just call this anyway :)
>
> Maybe you’re right. Whenever I see “CF” instead of “NS”, I think it’s old
> Carbon crap that isn’t going to work much longer, but that’s not true, is
> it?
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/quicksilver/Quicksilver/pull/943/files#r1001016
>
This is meant to act as a warning, not fail.
I prefer to "import" parts of the namespace as opposed to complete branches, i.e. `ABC = Furnace::AVM2::ABC`.
I have mixed feelings about this - the CSS similarity is nice, but the syntax gets uglier due to requiring quotes for the key, and the analogy breaks down for complex cases since you can't combine selectors the way you can in CSS. For example, it would be elegant if we could replace highlightCircleSize with :highlightRow { circleSize }, but then there'd be no straightforward way to set a separate size for a point covered by both :highlightRow and :highlightSeries.

For the per-series settings, the paranoid part of me thinks that they'd be safer in their own separate namespace to avoid conflicts with built-in names. For example, the data source might decide to name a series "strokeWidth" or ":highlight". It's not likely to cause problems in practice though.

Anyway, let me know if you want me to rename it. Should it be ":highlightSeries" to avoid confusion with the existing data row highlighting?
does this class not do anything, or any additional builder? Maybe would should use a common class for a default builder.