labofsahil/github-event-dataset-2012 huggingface dataset:
So a local transaction which has locks on local keys but does not have remote locks is not going to be transferred although these local keys now also belong to a new node. This is wrong.
sorry! ;]
`$breadcrumbs` is not defined. It will throw notice.
there is no fourth argument in ``compileAsset``
i think we should not have commented out code except in very special situations. i would remove this if its no longer needed.

and indention is off (not sure about the intention :-)
Could you remove the the space character between `[` and `0`?
This name isn't right but I'm not sure which page this is page object is for!
Sorry, I didn't mean to delete it.  Re-added now. :)
Good catch! Addressed in eb1760b
Can we just call them lists ?
something is wrong here whitespace wise - indention's in shogun are tabs of size 4
Ok cool. And every ab test will be created through that method?
Unused went away.
the pythonic way is `if netPropagate is None`. `None` is a singleton, so the more-efficient `is` check can be used
Is it possible to add a ``@param`` annotation for parameter ``$className`` which defines a more specific range of types; something like ``string|array``, or ``null|MyObject``?
Interest yes, time hardly.
I don't think Elixir support this.

@josevalim: Can you confirm, please?
Should this be declared as final?
See the benchmark on `#blank?` up there - I love the terseness of `#present?`, but it drops us into Ruby code for comparisons rather than staying in C.
We don't use JError in the platform anymore, instead use the JLog workaround. (There are a number of examples for this)
I'd remove the mkdir part. I'd let the calling method deal with creating the folder.
Do you mean

    'defaultgroup' => ($group) ? $group : (isset($this->option) ? $this->option : JFactory::getApplication()->input->getCmd('option')),
should use `.set('html')` for all the `table` and other weird cases.
If the user unchecks the "Allow repacking" option it always reverts back to checked when there is an error in the form. This is also why the template line that deals with it has also been edited.
That's fixed now. Thanks for pointing it out.

On Fri, Sep 21, 2012 at 11:59 AM, Rory <rorydriscoll@gmail.com> wrote:

> Oops. Thanks. I'll do that.
>
>
> On Fri, Sep 21, 2012 at 10:30 AM, Fredrik Ehnbom <notifications@github.com
> > wrote:
>
>> In sublimeclang.py:
>>
>> >      try:
>> >          if len(tu.var.diagnostics):
>> > +            print len(tu.var.diagnostics)
>>
>> @rorydriscoll <https://github.com/rorydriscoll>, looks good save for
>> this print. Could you remove it please?
>>
>> —
>> Reply to this email directly or view it on GitHub<https://github.com/quarnster/SublimeClang/pull/133/files#r1663647>.
>>
>>
>
>
I would use the same PointCloud2 message, but add additional channels with the features you need. As channels are dynamically defined the message format does not need to be changed. That way, information is kept together and you can always write a rviz plugin to display the additional information, for instance display the identifier in your case.
Wasn't using that non-canonical anyway, so I've removed support.
If you pre-define these variables at the module scope, you can get rid of these global statements in any function that does not assign a value to the variable (I think all functions but main?).
`$('#id_password2').keyup(checkPassword);`
No, user exposed strings we have in `"`. That's the main difference. The idea is that you can fast search for all strings which are visible to users. And other strings which are used internally.
I wouldn't write it here before being sure and testing it :)
Write out all arguments callback functions receive. In this case, `event`.
Probably no real need for a variable here. Could be simply in the for loop. (Just hint, no need to change, but you can.)
OK, this signals  that this is a bit too complex function. ;-)

I would suggest that you all this move to `get_context` method of the panel.
You can use `jQuery.each` to iterate over arrays. (I find it more readable.)
Would be great to make a decorator to do this, since we have the same code for each return. Can be done later though.
For PSR-2, the catch belongs on the same line as the brace closing the try block.
this is a hack, find the real reason why the spell is not working.
"callback"
This seems fine, although you might want to make it a little clearer that it's optional, and/or that minor failures aren't necessarily big problems.  

Basically, I just want to make sure users don't run the tests, see some small platform-specific error in a package they aren't going to use, and throw up their hands saying "These Astropy people are totally incompetent!"
entries and their synonyms should be added to the ns.php file, use the MapQName function to automatically get the canonical ns
This one is not trivial, `\` in double-quoted strings must be double escaped, or it will represent a special characted.
back to `image` field, use ImageField instead of FileField
I didn't understand why this change in the EHCache dialect is required.. why?
Compilator said me the same thing ;)
Deprecated, sorry for my english.
Nah, nevermind. This works. What is our schedule for this reorg?
I changed the API on purpose (and I think it's OK since this library is pretty new) because the DateTime object is pretty confusing in my opinion. You can't tell if 2011-01-01 is really the date MusicBrainz has or if it's just has been arbitrarily completed by the library you're using.
And I know that as a user I'll more often use a PartialDate than a DateTime, furthermore it implements the Comparable interface.
I know this is a legit thing to do in Django (refer to a model by a string) and it makes sense when you're referring to a model in the same module, but when you're referring to an imported model, I prefer explicitly importing it and referring to it directly. That way, if you're looking for dependencies, it's just a little simpler. Not a giant deal, but a stylistic thing.
My tests are failing and I think the paster commands will also fail. The celery tasks just log the exception but as far as I recall will not continue with the rest of the code.
Newline before
@bnoordhuis 
> What kind of filters are you worried about?

The code in mod_pagespeed is: http://code.google.com/p/modpagespeed/source/browse/trunk/src/net/instaweb/apache/header_util.cc#93

In Apache if we set a cache-control header we disable mod_expires and mod_headers (fixup_headers_out) for this request.  Talking to people here, the reason we're doing this is a little complicated, but basically the headers pagespeed produces shouldn't be changed.  Imagine someone has set all css files to be cached for 10 minutes and all images to be cache-control private.  When we do our resource fetch (for style.css or image.jpg) we'll see those headers set and respect them.  When we serve the rewritten resource, though, we need to modify those caching headers to change the cache lifetime (because the url includes a content-hash) and we don't want that undone.

An example.  CSS files are set up to be cached for 10 minutes.  Someone requests `page.html` which includes:

      <link rel=stylesheet href=/style.css>

Pagespeed will fetch style.css from the server and see the 10 minute cache lifetime.  Then it will respond with the page rewritten as:

     <link rel=stylesheet href=/style.css.pagespeed.ce.LyfcM6Wulf.css>

The browser will then request `style.css.pagespeed.ce.LyfcM6Wulf.css` and pagespeed will tell nginx that it should be cached for a long time (1year?).  We don't want that cache lifetime to be reduced to 10 minutes because that would break the whole point of cache extension.

I don't know how people manage caching headers in nginx, so I don't know if that would also be a problem here.
@jeffkaufman:  wouldn't __sync_add_and_fetch suffice?
You're right: good compilers do. I'm not 100% sure I can qualify MSVC as being a "good compiler"... :)
Why are you using LOAD_WITH_ALTERERD_SEARCH_PATH?
Do you think it's worth using `enumerateKeysAndObjectsWithOptions:NSEnumerationConcurrent usingBlock:` on this?
Not really related to this commit, but it seems like this instrumentation might actually belong in the `FactoryRunner`.
Will do.

What is wrong with python json library? Will not do unless there are some gains.

Because it was developed independently. Will might do.
User should understand what has happened.
Certes. En attendant cela permet quand même à un exploitant de déployer avec un simple "ant initportal".
Okay, I'll do it today while I'm traveling.
A question which is slightly more general than just this branch: in some python scripts we import numpy, in others we import numpy as np.  Might be good to choose one and stick with it?  (It's a reasonable mix of the two, when considering this script, the various galsim/*.py, and tests/*py)
(and also like the idea of storing the CM, will implement and let you know when pushed...)
shouldn't it be key.name.equals( ... ); ? I always get confused about checking equality on immutable objects.
Ok. I actually do that in some other files and I do indeed like it better.
Interesting, didn't know that this worked in python.  But I confirmed myself:
```
>>> AWS_QUERYSTRING_EXPIRE = 3.156e+8
>>> print AWS_QUERYSTRING_EXPIRE
315600000.0
```
I hadn't thought of drawing the line at 'token-type things' versus 'arbitrary strings'. That's interesting. In my mind, I've always thought of the difference as between things I expect to mutate and things I don't, with mutable things getting double-quotes. This seems to aesthetically go along with the escape character interpolation of double-quoted strings.

Since this is returning empty string not for the purpose of seeding a construction but to signal 'nothing here', the version I think of as less-mutable seemed appropriate. And I think '' looks nicer than "".

But in this specific instance, it was habit left over from being the keeper of someone else's coding standards; I'm not really religious about this (though I think having an established norm is useful.)
it looks like you're only adding the 'ready' instances of everything, is that correct?  might want to also add the 'draft' version if we want to display things as children in edit mode.
Same thing here, it appears. editProfile() needs a transaction for testing whether the display name exists and writing it to ObjVisitors if it doesn't. Otherwise, two users could select the same user name, both calls to editProfile find that it doesn't exist, and then both calls to editProfile write that same display name to their respective ObjVisitor objects. We can't let that happen.
this can be in the interface, just make sure you don't set = null in the body of the descendant class.
This is the domain object class (`*Do.java`) -- the DTO's are in 
`*Dto.java`.  Which is the right place.  It is messy accessing 
package-private fields I agree -- but *every* solution in java to the 
DO/DTO split is messy.  The solution I went for was to expose only a 
small set of external functionality:  essentially `BrooklynCatalog` and 
the .XML format, and allow the internals to be fast-and-loose but concise.

If this structure is correct we can harden the implementation and create 
duplicates without transients, immutables, etc, if we want.

--A


On 26/11/2012 12:14, Aled Sage wrote:
>
> In core/src/main/java/brooklyn/catalog/internal/CatalogClasspathDo.java:
>
> > +
> > +    boolean isLoaded = false;
> > +    private URL[] urls;
> > +    private CatalogScanningModes scanMode;
> > +
> > +    private CompositeClassLoader classloader = new CompositeClassLoader();
> > +    private boolean classloaderLoaded = false;
> > +
> > +    public CatalogClasspathDo(CatalogDo catalog) {
> > +        this.catalog = catalog;
> > +        this.classpath = catalog.dto.classpath;
> > +    }
> > +
> > +    /** causes all scanning-based classpaths to scan the classpaths
> > +    * (but does _not_ load all JARs) */
> > +    synchronized void load() {
>
> I don't think the DTO class is the right place for this method. It's 
> going far beyond what a DTO normally does (i.e. used for transfering 
> data). I think it would be better to move the business logic / load 
> etc somewhere else (but not sure where yet).
>
> —
> Reply to this email directly or view it on GitHub 
> <https://github.com/brooklyncentral/brooklyn/pull/396/files#r2221001>.
>
Have two constructors:
public ```NewAttachment(string url, string name)``` and ```public NewAttachment(string url)```
I believe `"test/"` is better than  `"test"`here, if it's a reference to `test/index.html`.

When those files are online, the web server adds `/` automagically if it's missing after a subdirectory name, but that requires a redirect and a subsequent additional HTTP request. May cost about ½ of a second in a satellite-involving or a pre-3G mobile network.
`return typeof obj === 'number' && isFinite(obj);`
correct.  For some reason I was thinking of `hasOwnProperty(...)`.  Although I'm not sure what the value of copying a property that has an undefined value is.

(Or, for that matter, of avoiding the `if` check entirely and just setting properties with undefined values where they don't exist).
this should probably be a proper `package.json`, if that's what it's trying to accomplish.
__2.0__ uses PHPUnit __3.5__, this assertion was introduced in __3.6__.
How can I send a message through UI? Have you tested this?
oh, so right now /users/ doesn't work?
This is still incorrect. Also if you keep winking that much I'll assume you're flirting.
If $client is public, why do we have getClient();
This requires to always have a PartsBin available?
It was already there on line 23
Ok I'll make it an attribute.
Не совсем понятно это условие. Т.е. если на вход дать "1" - программа напечатает "Nothing input."?
Боксинги-анбоксинги лишние (хотя, конечно, код чуть проще получился).
Нет. Обсуждалось на семинаре, в публичный доступ не хочется выкладывать фамилии.
Формат программы специфицирован. Не надо добавлять слова от себя.
Where is the unit test for this change?
I don't think linking to Persona's CSS is a good strategy. It will introduce deployment coupling between the projects. We're better off making a local copy of these files, so core can move at it's own pace and rename files as needed.
As mentioned, let's not do this.
Again, I would suggest either a list of device types, or perhaps in this case a dictionary?
Yes the comment suggests that you are checking it contains applications but the Assert itself on line 28 does not check the number of applications.
We should add the '_locator' suffix after the name.
Sorry for my negligence
Well that would be why Travis is failing, will restore fosrest to comopser.json
a unit test=Unitテストは用語集に追加しました。もともとの翻訳部分でもそうなっていました。
I've now added a non-windows implementation of the logging.
You should use a proper rfc822 quoted-string here (which requires backslash-escaping certain characters) otherwise you're just reducing the set of problematic filenames instead of eliminating it. You can use `email.utils.quote()` in the standard library for this.
Or you could just store your netbeans crap elsewhere.
No, the peak finding in hough space needs some special treatment, which is caused by the nature of the angles (continuous). Two 1D maximum-filters with "wrap" mode would not work here - the relevant part is in lines 225-232.
`assume_unique` doesn't work on my box (I'm still with scipy 0.7...).
if it's not a bug it should be consistent across fit methods
Where do you see that blackberry-deploy is only ip address? I'm looking at the usage output:
````
<device>       - hostname or the IP address of the target device or simulator
````
plus I've tried it. It spits out an error if you give it a bad ip or if it can't resolve a hostname, but it works fine with a resolvable hostname.
Appcelerator coding standard requires testing for NULL, e.g.,

if (timer_ != NULL)
This doesn't need to be in self
Is it useful here ?
`@Spatial(name = "location", gridMode = true)` 
`<literal>@Spatial(name = "location", gridMode = true)</literal>
I was more concerned that the if statement would effectively be

    if (SESSIONREPLYTIMEOUT)

since the test is a comma expression.  If it isn't ever called then that explains why it isn't a problem.
potential for commoning this code with code elsewhere into a function.
This will clash with another pull request by Adrian. He's seriously revamping how reportlist and query can use the same form base.
See https://github.com/mozilla/socorro-crashstats/pull/186

Whenever you muck around with default parameters to form classes you run the risk of accidentally assigning the wrong parameters. 
It's always safer to do `form = MyForm(data=request.GET)`
You forget to assign values to your keys here. Add ``... % params`` at the end of this line.
This is an absolute nit and more of a "tip".

It's better to use this notation:

```python
attributes = [
    'avg_process_sec',
    ...
]
```

Because if you decide to change he variable from `attributes` to `attributes_in_data_dict`, to conform to pep8 you're going then also need to change every single line to come thus making diffs harder to read.
you could try `awk '/^#{@new_resource.package_name}: /` to be more true to the original, but still work with homebrew 0.9.2
Why?  self class should do the same thing.
If we leave the loadFinished signal connected to the page, this will happen automatically.  Any reason this was disconnected?
yes, these are necessary as of the dots in the corners need more space
Should this be ``None``?
@yuhunglin  Can we use as many full names as possible? e.g. functionMap & bspaceFavoritesDeferred?

Also, my personal preference goes to 

```javascript
var functionMap = {
  'filterOnCategory': calcentral.Widgets.bspacefavourites(favWidgetName).filterOnCategory,
  'loadFavouritesList': calcentral.Widgets.bspacefavourites(favWidgetName).loadFavouritesList,
  'loadCourses': calcentral.Widgets.canvascourses(canvasWidgetName).loadCourses
}
```
wdyt?
I guess we should not delete any methods even if they are never used. 
Even if they are in private class and even if they are private.

But I hope it is not a problem for us if you try to submit that to Apache.
Is this just to satisfy a warning about having a required comment?
Sorry. I know this is off topic but without this you can't pass extra arguments to `./runtests.sh`.
Thanks, I'll change that too.
To be more specific isn't this just?

    file { '${directory}/conf/razor_server.conf':
      source => '${directory}/conf/razor_server.conf.default_workaround'
    }
Also, please don't ask questions in comments.
It's not a big deal: it's clearer to me, but might not be for others. I just saw the `u/2` directly in the `A`-index and it wasn't clear to me why. I just tried to make it explicit that the x-equation starts on one column and the y-equation starts at an offset w.r.t. that column. Take it or leave it---it's probably not that important.
The `do/end` isn't necessary - you can just do `@model = define_model(:example, :attr => :string).new`
This function can be simplified to 

```javascript
return this._visualizers.slice(0);
```

unless for some reason you need them to be in reverse order.
What about tests for all the exceptions?
We should change `project` to take a result parameter, but that can probably wait until a separate pull request, I plan on doing more cleanup like this sometime soon.
I disagree.  Please maintain whitespace and clarity here.  I don't believe .php file size trumps readability and maintainability for anvil .php files.
I had more complex polling in mind, e.g
start with 250ms interval, if no input snippets, change to 500ms. when when input snippet, change back to 250ms.
But what you have right now should be fine too.
Might be clearer to say, "Same as the normal background color of this view."
Are these the accelerations of the frame center of mas? I think they should be define from the frame center of mass to be more generic and you shouldn't have ``s1`` and ``s3`` in this equation. Move the conversion from the acceleration of the vectornav point to the frame center of mass into BicycleDataProcessor because it is only specific to a certain bicycle.
Typo fixes.  I meant: *The actual throw would occur where it does now, but we'd also do something like:*
Please add a space before the "{". Same for many other places in the code.
I would prefer a

 if(pc._signaling)
     pc._signaling.close()

style... It's more homogeneous with the rest of the code and you are not catching exception for nothing.
I hear ya.  I also like that the original version *only* talks about callback ordering, and doesn't introduce yet another "If promise is fulfilled/rejected" phrase into the spec--in general, I feel like that phrase is problematic.

>2.2.2. If onFulfilled is a function,
>>2.2.2.1. it must be called after promise is fulfilled, with promise’s value as its first argument.
>>2.2.2.2. it must not be called more than once.
>>2.2.2.3. it must not be called if onRejected has already been called.
>
>2.2.3. If onRejected is a function,
>>2.2.3.1. it must be called after promise is rejected, with promise’s reason as its first argument.
>>2.2.3.2. it must not be called more than once.
>>2.2.3.3. it must not be called if onFulfilled has already been called.
How do I fix that? Can't find a way to edit this (sorry, pull request n00b ;) )?
whoo-hoo!
Any reason we have location and venue name being displayed?
Кстати, сейчас могут запросто быть клеши с другими плагинами. Так что согласен, что нужно как-нибудь переделать. Я за `uploadcare-dialog-open.uploadcare`.
Same comment as above applies here.
I see, thanks. So we have a sort of perfect forwarding implementation here. I'm still concerned about its narrow use and unintuitive syntax.
`FILE*` and `File` aren't file descriptors. "File descriptor" is a POSIX term which refers to OS-level file handles, i.e. indices into a table of open files for a process. Quoting [Wikipedia](http://en.wikipedia.org/wiki/File_descriptor),

> The FILE * file handle in the C standard I/O library routines is technically a pointer to a data structure managed by those library routines; one of those structures usually includes an actual low level file descriptor for the object in question on Unix-like systems. Since file handle refers to this additional layer, it is not interchangeable with file descriptor.

`isFileHandle` would be better, but I'm not sure if that is 100% appropriate either.
this call should also work without an explicit instantiation
I'm seeing inconsistent use of operator spacing, even in the same line. I'll use this as a pretext to require spacing around all operators, like in the rest of phobos.
Again, if folders are inserted one at a time, I think we should bring them as early in the process as possible, and do it directly by calling `insertFolder`, rather than `enqueueRecord > enqueueFolder > incrementalFlush > flushReadyToWrite` etc.

Just call `insertFolder` at the top of `recursivelyAddRecordAndChildren`, which will avoid the lies in that method!

My suggestion might change if we actually did bulk insert folders, and could do so safely, but given that that's not the case, I'm inclined to have `readyToWrite` only ever contain non-folders, which will simplify this method significantly.
Wrap this block in `if (Logger.shouldLogDebug(…)) {}`.
You should probably compare with `PREF_INVALID` according to https://developer.mozilla.org/en-US/docs/XPCOM_Interface_Reference/nsIPrefBranch#getPrefType%28%29, instead of relying on the fact that (if I understand correctly) `PREF_INVALID` happens to be falsy.
We might as well stick to the `d3.v2.min.js` version, instead of the un-"minnified" version of the library. We're realistically not going to look at the source anyway..
Why declare 2 parameters but only pass in one?
@beberlei why is there no url as ``docs.doctrine-project.org`` for this one ?
Estas creando un nuevo Factory por cada asiento, cuando no es necesario
Ich habe zuerst gedacht, diese Update - Services einfach im Controller hintereinander aufzurufen (-> das gibt eher Datenleichen, oder?)
Ist auch ok, wenn ich einfach die updateFirm & updateAddress noch am Ende des CreateFirm Service reinschreibe?
If anything deserves a comment, this does.  This just doesn't give me any sort of "warm fuzzies"....
another blank line here
No, it only needs to be a unique string on each call to the function.
The ID is used in sakai.api.server.bundleRequests. 
Looking at this function again, it could be doing a batch request itself, instead of using the bundleRequests function.
indentation should be just four spaces. backslash is not needed for line continuation because you're inside a pair of braces. And if the contents of the braces extend over more than one line, the final end-brace should go on its own line so its easier to visually match up with the opening brace.
Would it be possible to use D_LP64 instead?
Also, if Spring Redis adds a new type, we'd have to add it here; whereas with a simple string, it should just work without changes.
Not keen on 'underlying'.

actualResource ?

Also, needs to be

````java

message = this.source.receive();
if (this.isPseudoTxMessageSource) {
	Object actualResource = messageSource.getResource();
	resource = actualResource != null ? actualResource : resource;
}
````
Nit: Why did the .queryEntities move to its own line? The before looks cleaner, IMO.
`$('<option>', { 'value': metric.id })`??? This works? Vau! I didn't know that.
`this.serializeData()` can return data instead of a deferred and jquery will recognize this and pass it to the then() handler. This will preserve backwards compatibility with existing implementations of serializeData while supporting async as well.
Move styling to base, because can be used in openlayers as well.
It's done, I have revert the modification.
I think I will ask to Chase in the same time with the other future modification ?
I don't think we should add an opt in field to subscription list since it would only be used in UpdateSubscriptionInfo.  maybe instead of the <string, bool> dictionary in update subscription info, we could have a list of {SubscriptionList, bool OptIn } objects if you want to strongly type everything.
Is it necessary to do "print" in this stage?
Do we care to wait for observers to be done or not really?
I'd ultimately like to use a logging framework, but none of that is implemented in the OAuth libraries at this point, so I just went with the current pattern.
I dont know if this is an issue or the expected result :S should we fill a bug?
I'm assuming this is the same for ML, the method we made (as someone pointed out before) can be a bit misleading. It should probably read `[NSApplication isLionOrHigher]`
I like that. We could also use `EntityIndexingInterceptor` which completely defined the feature.
You could, but these classes are used to parameterise a template class, and you want them to be as efficient as possible.
Yeah, I'm just proposing the bare sort-of-working version.   If you want to get fancier, that's fine, but we definitely want the MX lookup to have some value.  It's very unlikely the MX entry will ever point at something that will be in our ISPDB.

Also, I obviously meant for us to invoke .join('.') on the end there.
This method does not exist in android.
Not affecting documentation, but while the issue is open, do you think it'd be good in the future to make this paramater optional in order to gain parity with Android and MobileWeb? Alternatively, allow Android and Mobile to accept the current window as a close argument?
I would suggest to move the edit test in a new directory so that we can test only that feature.
I understand. Thanks for working so late for code review... I know this is brutal.
``>2.2.*`` is wrong. you cannot mix operator-constraint and ranges in the same part. It simply does not make any sense (it would basically mean ``>(>=2.2-dev,<=2.2.x-dev)``)