labofsahil/github-event-dataset-2012 huggingface dataset:
I'd like to see these pieces closer to the top.  When we get closer to done, I'll look at doing an intro paragraph that provides a bit of perspective for the mountain to be climbed, and lets people get a sense of which waystation they need to reach.  I'm still convinced that providing reachable steps between what we teach in the installfest and workshop, and even modest git mastery, is critical for bridging students and volunteers into becoming effective developers.  I'm also convinced that we need a learning back-channel to get feedback on how we communicate to those who aren't yet through the installfest or workshop, because if we lose them there, we've generally lost them completely.
Should I provide a way in `Veritas::Relation::Header` to test if an attribute exists?
Thanks for the contribution.

I'm a vim user too, but I prefer to keep non-project-specific ignore patterns in my global `.gitignore` file.
is it right that  factor "dose" cannot exist without "compound"? it might be for example dose of radiation.
Add a "Returns" section and a "Raises" section.
This method does not appear to be private.
Document this global.
I get app loadbalancer here confused with our AppLoadBalancer - why not just `write_app_config`?
Don't mention Chandra, just state why things are the way they are.
Remove this extra newline as well as others in this method.
Check why this is needed?
Actually, on closer look `get_schema` is either going to return `{ok, Schema}` or throw.  So either turn this into a catch or just take the happy path.  Personally I would vote happy path for now as `get_schema` should almost always return.
seems like a good idea. i've made all the changes you requested and adjusted the pull request
debugging print?
@pborreli no. Composer does not use this rule of the capital ``B`` which confuses the IDEs (they don't recognize it as boolean but as a class name).
I don't think this directoryIsEmpty check is needed (neither below), there shouldn't be a composer.json in the directory you're trying to create a project in. It can be completely empty or it should throw an exception IMO.
Because it's an assignment. It's a best practice. Google it :)
this phpdoc is wrong as there is no use statement for the class, meaning it will be resolved to the wrong class name
Heh indeed, thanks.
refactor to isolate parts of code which need to be overridden
j'aurais mis une virgule  : 
```
Par contre, peu de logiciels de graphisme proposent cette option
@randx I'm not sure what you mean by this..?
I think it's so that non mustard cutting bowsers get embedded tweets styled
(briefly discussed with Patrick). If that's desirable, I think this is an
OK approach - although we should move it to the bottom of the page so it
doesn't block browsers that don't support async and defer attributes.

But, maybe if you don't cut the mustard you shouldn't get the standard
tweet embed?

On 18 Oct 2012, at 08:06, Matt Chadburn <notifications@github.com> wrote:

In article/app/views/article.scala.html:

> @@ -1,17 +1,16 @@
>  @(article: Article, storyPackage: List[Trail])(implicit request: RequestHeader)
>
>  @main(article, Static, Configuration, Switches.all){
> -    <link rel="stylesheet" type="text/css" href="@Static("stylesheets/article.min.css")" />
> +    @if(article.isLive){ <script src="//platform.twitter.com/widgets.js" async charset="utf-8"></script> }

What does this do and why is it not loaded in the same way as the other js?

—
Reply to this email directly or view it on
GitHub<https://github.com/guardian/frontend/pull/141/files#r1875614>.

Please consider the environment before printing this email.
------------------------------------------------------------------
Visit guardian.co.uk - newspaper of the year

www.guardian.co.uk    www.observer.co.uk     www.guardiannews.com 

On your mobile, visit m.guardian.co.uk or download the Guardian
iPhone app www.guardian.co.uk/iphone and iPad edition www.guardian.co.uk/iPad 
 
Save up to 37% by subscribing to the Guardian and Observer - choose the papers you want and get full digital access. 
Visit guardian.co.uk/subscribe 

---------------------------------------------------------------------
This e-mail and all attachments are confidential and may also
be privileged. If you are not the named recipient, please notify
the sender and delete the e-mail and all attachments immediately.
Do not disclose the contents to another person. You may not use
the information for any purpose, or store, or copy, it in any way.
 
Guardian News & Media Limited is not liable for any computer
viruses or other material transmitted with or as part of this
e-mail. You should employ virus checking software.

Guardian News & Media Limited

A member of Guardian Media Group plc
Registered Office
PO Box 68164
Kings Place
90 York Way
London
N1P 2AP

Registered in England Number 908396
options like `watermark_options[mask]`will also include the `ABSPATH`, can we do some recursive replace?
per flow integer
These can go back to being `NULL` with the changes discussed above.
minor style thing:  if event[field].is_a?(Array)    # don't need () around the main if expression
I'd remove the "Sign in or". I think just "Sign up now!" is better, since current users will probably not see this anyway.
Nit: We put spaces after brackets and try to be consistent in quotes at least with in the file.
I'll add a fix.
I would like this to load the same way we load other page's code, using the code in start.js.
I had to change all your 'foo' to 'lang', since 'foo' is not a whitelisted key :)

Out of respect for the canonical foo - bar - baz sequence, I have additionally changed all bar to foo, and baz to bar.  Because I care.
It's valid, you probably misread that part. If the element doesn't have no content the end tag can be folded into the start tag.
I will trust those styles because I cannot test it in the next time.
Any reason this isn't just in a `margin` declaration?
I don't think you use this import anywhere in this file ...
You can't use CommndPreprocess here since there isn't a command.
ah crap this test is busted.  that explains some things.
Please add copyright/license header information, see other files as an example.
statsDumpInterval
bad news, this is all in the wrong file, I just noticed :D this needs to be in lib/reporters/html.js

also these two functions don't need anything in the closure so I'd move them lower in html.js by `on()` etc
we can use the "%s" printf-style specifier here instead of concats
Could you refactor this? Is hard to read
Another test I might suggest would be to check if the form errors come through during a validation failure.
Space between cast and variable.
@Maks3w  : ok, marked as skipped now.
This change fix ZF2-325
Typo `Call`
@spako Will fix it once it is merged in, will fix using github readme editor.
What is bad in this:
```ruby
@lesson.labels = extract_labels()
```
In compiled language compiler (optimizer) will do it for you.
In interpreted language it's your problem.
But speed is not the only concern. You introduce variable that is not needed, it makes code lengthier and more difficult to understand.
what is the convention guys? upper case or lower case?
We need to filter out unlisted packages (where there are no listed versions).  There's a strange difference right now where the home page shows one number and then you click on the Packages link and see a lower number.
Mhm. Wir werden dann unsere Server upgraden, damit wir 5.3.10 oder höher haben.
is there a js-fixtures global being leaked?  Why does this need to be defined?
Yura,

Amy and I have made the revisions you requested. Basically, we added a selector 'oocss' to the second div in PlaceTemplate.html, instead of using the JS selector that was already present. We also did some minor cleanup. Tests all pass (no reason why they wouldn't).

I've pushed the commits to my Github rjaffe/ui repo. From what I can see, they have automatically been added to the pull request, so you should have access. Please let me know if you need me to do anything more.

Thanks (hooray!),
Rick

On Apr 10, 2012, at 1:52 PM, Yura Zenevich wrote:

>> +.media { width: auto; }
>> +
>> +.size1of6 { width: 16.66666%; }
>> +.size5of6 { width: 83.33333%; }
>> +.smallfield { width: 75% !important; }
>> +.spacer { height: 4.5em; }
>> +
>> +.csc-place .cs-structuredDate-popup {
>> +	width: 615px !important;
>> +}
>> +
>> +.csc-place .nameDate .cs-structuredDate-popup {
>> +	right: 38px !important;
>> +	top: 127px;
>> +}
>> +
> 
> Hi Rick, Just started reviewing your pull request. I noticed you have csc-... selectors which are used as selectors in javascript. If you want to add just styling selectors I would suggest using cs-... Also I would change then from being record type specific, e.g. cs-record or something (maybe cs-authority-record). We are trying to move away from having record specific code/css in master so this should really be record-type agnostic.
> 
> --- 
> Reply to this email directly or view it on GitHub:
> https://github.com/collectionspace/ui/pull/21/files#r667638
not a big deal for now but this would be a nice separate component
Just realized you can replace the two lines above with this one:

```python
if item.get('app$control', {}).get('yt$state', {}).get('reasonCode'):  # Is it private?
    continue
```

The comment is to explain what this is checking for since the variable names aren't intuitive.
A bit fugly with the newline in the debug output. Why no two calls? It's slow already :-D
Can you explain more? I've never heard this before. 

This SO says not to use bang methods on controllers, but I'm not convinced: http://stackoverflow.com/questions/1761076/when-do-i-use-save-create-and-update-attributes-in-rails
```javascript
function classA() { /*..*/ }
var classB = classA.bind(null, "x");

classA.prototype.lolcat = "=^__^=";
classB.prototype.lolcat = "=°__°=";

```
space at end of line added here
Yeah, this is a result of using an older version of jingo that required it. It will be changed post-FB-app-release when the entire Affiliates site gets updated to the new playdoh and updates all of it's libraries.
s/sms_page/SMS_page
s/Sms/SMS
Is it necessary to prepend with the en-US locale, here?  Having that redirect coverage would be nice, if it's possible.
lockPath is already String
Well, it works, and it complains if it doesn't work. I'm not going to re-implement that particular thing.
Obviously, it is a complex piece of code that can take any GPR string and convert to our protein data structure. This does it, and it works when applied to an entire genome-scale model. And it works right. Not sure what else I have to say here.
Chris
On Oct 22, 2012, at 5:58 PM, Scott Devoid wrote:

> In lib/ModelSEED/MS/Factories/SBMLFactory.pm:
> 
> > +			$proteins->[0]->[0] = $proteinItems
> > +		} else {
> > +			foreach my $item (@{$proteinItems}) {
> > +				push(@{$proteins},$self->parseSingleProtein($item,$gprHash));
> > +			}
> > +		}
> > +	} elsif ($root =~ m/\+/) {
> > +		$proteins->[0] = $self->parseSingleProtein($root,$gprHash);
> > +	} elsif (defined($gprHash->{$root})) {
> > +		$gprHash->{root} = $gprHash->{$root};
> > +		return $self->translateGPRHash($gprHash);
> > +	} else {
> > +		$proteins->[0]->[0]->[0] = $root;
> > +	}
> > +	return $proteins;
> > +}
> 
> What does $gprHash look like?
> What should $proteins look like? Especially that triple-deep array ref...
> At each if statement, what do we find out about the gprHash?
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
Song.requestable is a property, not a method
multiplayer? :-D
this is why the parameter has a default dependency. but i can change it to use a setter
this message tells that the file on the DFS cannot be accessed, but the $srcFilePath is a local file path, passed to the function copyToDFS. these lines should be on the copyFromDFS function. 
otoh, the hunks is declared as: 
@@ -95,19 +95,41 @@ public function copyFromDFS( $srcFilePath, $dstFilePath = false )

so, right function, wrong lines? this hunk has something wrong
if $attribute.name contains a ", you'll get a JavaScript error. You should add |wash( 'javascript' ) to avoid this.
`code` is already a numeric status? So just use it as an argument to `status`?
Food for thought: http://dablog.rubypal.com/2007/8/15/bang-methods-or-danger-will-rubyist
This should be done in the HomePage constructor...
Is "include Object::Singleton" needed for ruby 1.9?  I've always just seen "include Singleton".
Perhaps there should be a test that de-serializing an older version (with file_url, file_url_expires, &c), is deserialized and keeps that information?
Is it really a good idea to put this on a face?  That seems awfully public for what is, ultimately, an internal utility?
Either this should be `when`, or the rest of the function should be the 'else' condition. I'd lean toward the former.
Surprisingly it does, NULL does not count as a value to be unique.
Need to specify the expected input format.
I think there is a logical error here. The callback will never be executed when the last module that is checked has no config file. Also, this should be looking at all files in /config, not just config.json
Most likely I'm missing something, but what's the purpose of having both `tpePresenter` and `typeOfExpressionPresenter`?
Damn old style classes. Yeah, I guess what you're doing is fine. Mixing in object seems more confusing.
No, just using time_get() can be a bad idea because the lower bits of it could always be zero depending on the timer resolution.
I'm not really a fan of creating module methods that just call the same thing somewhere else
changeCount - это количество нуклеотидов(элементов массива dna), которые необходимо изменить
Число будет равно 0 при rate == 0, и равно dnaSize при rate == 100
В лекции было так, но вообще лучше, чтобы такие объявления были в своем файле, здесь Cell+Mutator.h
@voleg да хоть BDD ))
Спасибо за комментарии.
Her'e's a bit of explanation of this: http://blog.markusproject.org/?p=3827
Should we wrap GROUPNAME/ with brackets to show it is optional as we don't have it for Brackets?
Nitpick... From a semantic standpoint, I don't think `FetchProgress` should derive from `IndexerStats`.
To my knowledge, indexing is only the last step of the fetching process.
Guidelines?
following mne.utils.check_random_state if rng is an int it's the seed. Was unclear to me what rng could be when reading this code.
maybe you can add more comments
Maybe but it's a huge API break and I think we can live with that now don't you think?
also


On Sun, Dec 16, 2012 at 10:18 PM, Alexandre Gramfort <
notifications@github.com> wrote:

> In doc/source/whats_new.rst:
>
> > @@ -90,6 +102,9 @@ API
> >
> >     - The artifacts module (mne.artifacts) is now merged into mne.preprocessing
> >
> > +   - Epochs objects now aldo take dicts as values for the event_id argument. They now can represent multiple conditions.
>
> aldo?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/mne-tools/mne-python/pull/221/files#r2430652>.
>
>
same here; iterate over all the results not just the one at index 0
These changes look reasonable, but I'm curious if they are needed or defensive. I would have expected that any code that got here would have a valid node object and these default values would already be in place. I'd say keep it as-is either way, but I'm curious.
Thanks you for noticing.  That was not intentional.  I created this change before that change was committed and so I was missing recent changes.  I have updated it to merge in those changes.
@guillaumebort  here is the reference https://github.com/playframework/Play20/blob/master/framework/src/play/src/main/java/play/libs/F.java#L350
Please reformat to:

```ruby
base_path = path_arr.slice(0 .. path_arr.index('metadata.rb') - 3).join('/')
```
What if `params[:username]` is `nil`?
i would add "with composer using --dev option"
Please use the right argument name (this is important as the next Twig release will introduce named arguments)
如果浏览器被识别为任何版本的MSIE
细节请不要忽略。
Assume that there is field name 'A' in doctype and same field name is there in child/table doctype. The doclist will contain both the doctypes and while calling the function "change_idx" for child/table doctype, as it is taking the main doctype and child doctype in loop, it will change the IDX value of both the fields as the condition becomes true based on only fieldname.

For testing, try adding the new field in parent doctype having same fieldname as in child/table doctype.
this is left by accident - there is no longer "blocking" mode
Дибаг-то выкинь.
no. the example template in the doc defaults to look at the ``messages`` translation domain. But translations for this domain can come from *any* bundle. There is no relation to the current bundle (the translator does not know from which place you call it, which would be totally wrong anyway)
What about `show`
I'm happy to do that, but why?  I tend to think the latter is more readable, since the rest is all buried in a string, and makes it look at first glance like we're returning a ruby string-of-code.
No need for the comment there
My apology. Said and done.
ya lets at least add a comment on this sucker.
:-) it is our code convention. I'll double check
why not `boolean`?
The tx is removed from the txTable in the lock breaking service.  

I set the rollback flag so the tx reference in the LockingInterceptors know this tx isn't valid so it will release the lock.
Deal! You are right
Got it :)

fixed by https://github.com/faenil/qmlgallery/commit/f173d3a780a80c2f170da7b8af19d46a4e6f4870
То есть мне теперь во всех проектах, где есть http авторизация, нужно будет в своем конфиге задавать ONPHP_TEMP_PATH ?
$message is a very common superglobal variable in PMA, are you sure you won't be overwriting and unsetting it here?
And is there really any need for this intermediate variable at all?
Specs are red under Windows. I pushed a new commit to fix.
the objective function in the paper (eq (1)) is really big and complicated. I tried to add some text description here and reference back to the paper. Hope this could be helpful.
It's the little, double bordered, top arrow. I should have added a comment ...
Fixed in 6e1e1ef.
Yes, you are absolutely correct, I've somehow missed negation in `if (!Object.isString(madeClipping))`.

And yes again, unchanged `if (Object.isUndefined(madeClipping))` should work. I've made here `isString()` just for symmetry with check at line 1427: `if (Object.isString(overflow))`.
in general, special meaning values are a bad idea, because no matter what you do, someone can always set the alternative to that.  even in this case, i could be like
```ruby
default AbTest.class_eval{@@UNSPECIFIED_DEFAULT}
```

not that anyone would ever want to do that
Trailing Whitespace!!! :)
Could you add ```.should be_kind_of(String)``` here?
I don't understand why we're continuing in the face of failure in this code: if any func() call fails, the checkpoint fails, right?  Why keep pushing forward?

It's also a little odd -- we quit if we can't get a particular key, but keep going if we can't get a particular btree, or the underlying func() call fails?

Ditto __wt_conn_btree_apply().
The assignment to ``$_COOKIE`` seems to be dead. Are you sure it is necessary, or maybe you have a typo somewhere?
you forgot to future-fact ? or is just the output of a previous failling test?
why duplication the same logic when all you need to have as difference is using ``$executor->execute($loader->getFixtures(), $input->getOption('append'));`` ?
Yeah, this variable `outdatedModels` looks like more like a comment, you're right!
> This is an interesting case, because it could be highlighted as markdown (since it is)

Github Flavored Markdown think different.
Solid point Sam.  I had forgotten that.
Just need to change to @since 12.1
if chaining is done this could be replaced by

    $session->initialise($this->input)->start();
This should be `!= 'true'` (XML attribute values are still strings). For testing purposes, use disabled="false" as value - this currently disables the form element as well.
it would be good to clarify that this means one has to register the gaufrette stream wrapper for this to work.
If all we're doing is returning the value, I don't think we need a `prepare_*` method.
И вообще это делает не то, что нужно. 
    //! Adds a new open instance id.
    //  Blocks if the open instance pool is full OR the
    //  reserved instance pool is full.
Блокировать новые открытые инстансы нужно не только, когда заполнился пул новых, но и когда заполнился пул зарезервированных.
Соответственно, схема с двумя condvar'ами работать не будет, придётся сделать одну. Это для предполагаемых нагрузок на этот пул не страшно.
Rephrase as something like "This method sets up a socket to the index, then passes an output stream connected to the socket to the run method of the ReceiverBehavior object. When run returns or throw an exception, it handles tearing down the socket." It's mostly the order of events that I'm worried about.
Nit: Static fields are typically declared before the constructor.

Due to your current implementation, you will want to declare these after `knownKinds` as well to avoid runtime errors.
It is necessary to handle also change so that copy-paste is handled.
And also in other direction. If you click on the venue in the list, you get the venue on the map enhanced (Google Map icon bouncing?).
Why `</img>`?
I am saying that you still have a stand-alone file with that in pull request. And that it is better to include that file at the end of `home.html` then to have content directly in `home.html`.
Why removed this line?
Constant string, comma.
I've changed string values with boolean values.
You should wrap more precisely. You should go into newline after `$('<span/>').append(author_link)`.
`provider_month_names`
*Password* lowercase?
I'd call it a build-time dep since it's not a runtime dep of imagemagick, but is used by things that call imagemagick config scripts during builds.
Я бы написал `def(process.cwd())` — так надёжнее, потому что семантика точки может в процессе парсинга других опций или в процессе работы приложения поменяться. Например, где-то будет вызов `process.chdir()`. А `process.cwd()` вернёт абсолютный путь до текущей директории ещё на этапе инициализации coa.
that's the content of my previous comment ^^
You are not calling UpdateCharacterNameDataLevel anywhere, are you sure that you ported the whole patch correctly?
Minor nit, could you change this and the other validation error call to have shorter line length like:
```ruby
format.xml { render :template => 'api/validation_error', 
                     :locals => { :errors => @pool_family.errors }, 
                     :status => :bad_request}
```	


```
Jap, aber das ist einen TextView.. ich hab den Titel grösser gemacht und entsprechend nur dem Titel grösseres Padding gegeben. Das Layout hat wie gewohnt 10dp.
sounds good. its low on prio right now



On Mon, Nov 26, 2012 at 10:26 AM, Jai Ram Rideout
<notifications@github.com>wrote:

> In scripts/subset_biom.py:
>
> > @@ -0,0 +1,99 @@
> > +#!/usr/bin/env python
> > +
> > +from biom.parse import get_axis_indices, direct_slice_data, direct_parse_key
> > +
> > +__author__ = "Daniel McDonald"
> > +__copyright__ = "Copyright 2012, The BIOM-Format Project"
> > +__credits__ = ["Daniel McDonald"]
> > +__url__ = "http://biom-format.org"
> > +__license__ = "GPL"
> > +__version__ = "1.0.0-dev"
> > +__maintainer__ = "Daniel McDonald"
> > +__email__ = "daniel.mcdonald@colorado.edu"
> > +
> > +try:
>
> Ok- so I guess we can merge this and create a separate issue to remove the
> dual-support from all scripts?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/biom-format/biom-format/pull/55/files#r2224740>.
>
>
Is this where you set the platform to use holo theme?
I assume you have now managed to install the app on your device?
OK I'll try that.
For consistency I would suggest that this property be renamed to something like kiji.test.standalone.
My bad:

$ circusd-cluster 
Traceback (most recent call last):
  File "/usr/local/bin/circusd-cluster", line 9, in <module>
    load_entry_point('circus==0.6.0', 'console_scripts', 'circusd-cluster')()
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 337, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 2279, in load_entry_point
    return ep.load()
  File "/usr/lib/python2.7/dist-packages/pkg_resources.py", line 1989, in load
    entry = __import__(self.module_name, globals(),globals(), ['__name__'])
ImportError: No module named cluster.cluster
Why are you using a parenthesis here?
We would handle this with the EventSupport-Mixindomain
I don't know why my pagespeed_automactic.a library lacks of this one, it does fix my problem when compiling:
Better to remove rather than comment out.
I'm not sure that you understood my intention for this code.  I need to search `virtualenvwrapper.sh` in `$path`, because there is no fixed location for this script, because it can end up in various places depending on how virtualenvwrapper was installed.  And that's how `source` works according to its [documentation](http://zsh.sourceforge.net/Doc/Release/Shell-Builtin-Commands.html#index-_002e):

> If file does not contain a slash, or if PATH_DIRS is set, the shell looks in the *components of $path to find the directory containing file*. Files in the current directory are not read unless ‘.’ appears somewhere in $path. If a file named ‘file.zwc’ is found, is newer than file, and is the compiled form (created with the zcompile builtin) of file, then commands are read from that file instead of file.

So in my understanding, `source virtualenvwrapper.sh` makes perfect sense, if I want to load `virtualenvwrapper.sh` from `$path`.  

To avoid an error message, I need to check whether `virtualenvwrapper.sh` exists somewhere in `$path` beforehand.  According to the [documentation](http://zsh.sourceforge.net/Doc/Release/Zsh-Modules.html#index-commands) `$commands` maps command names to file pathnames, and hence should contain `virtualenvwrapper.sh` if that script exists somewhere in `$path`.  Surely, there is no guarantee that the pathname actually exists, but how should a non-existing file end up in `$commands` during shell initialization?!

Anyway, that's how I see things, but I'm by no means a ZSH expert, and I'd be happy to learn how to do this check correctly.
I agree, it looks out of place
Actually never mind - that's not in the scope of this PR since that's from code that was already there.
No, it does not mean that. The override still works. This change means that the tests will work in the soapUI gui, which is important for developing the tests.
This comment should go up to line 615 to match the placing of the # Make the PSF profiles comment above
I think at this point, target_PSF is an SBProfile, not a GSObject, so I think this is right.
OK, I'll make the changes and inform when pushed.
Since the logging level is not a simple true/false option, might be useful to say what other reasonable options are in the comments?
On line 370 I wanted this to be 0.5 * lam_over_D / oversampling since it is very clear then (or at least is to me) that this is the critical sampling / oversampling for a pupil of the specified lambda over D.

However, where below npix is defined I thought I was being clearer, but I can maybe see how this isn't the case.  I've pushed a revision which defines simply:
```
npix = 1 + (np.ceil(pad_factor * (2. * np.pi / stepk_airy) / dx_lookup)).astype(int)
```
General comment on writing javadoc:
* See style guide at http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html
* Take a look at any of the java standard library javadoc (e.g. http://docs.oracle.com/javase/6/docs/api/java/lang/String.html)

Things like "This method..." is redundant; best practice is instead to start description with a verb.
Things like "that extends BrooklynCommand" is redundant, because you're describing an abstract method in BrooklynCommand.

So you could change this to:
`Executes behaviour specific to this command. Called by the framework after initializing all of the CLI option fields.`
Very minor: suggest moving `lastStatus.set(code)` outside of if-else, so that it will have value 200 if the run passes.
It should be `Character.isSpaceChar(c)`. I  rewrite it.
It might be better to have a bool indicating if `data.size` is set but this was the approach I took when I started the porting.
Can you pull out the pre tag around the link here?
Hi Thiago, 

Let us try to avoid the usage of the deprecated ``default`` template function. Could you replace with the following bit : 

```
{if is_unset( $width )}
    {def $width=500}
{/if}
{if is_unset( $height )}
    {def $height=500}
{/if}
```

Thanks in advance !
Yup. There is dialog function, and, as I remember from agents.coffee, there is some problems with using fat arrow in it.
probably better UnsupportedOperationException
UserGlossariesController にも同じチェックがあって重複しているので、ひとまず ApplicationController に移して整理しておきましょうかー
I find moving this code around highly suspicious. I remember swapping them this way to fix an issue. My git foo is failing on a short notice, but I'll have another look tomorrow. We need to be really sure that this doesn't have bad influences.
5px sounds fine on desktop but will be probably very small on device, 20px sounds better there (and will be consistent to the value use for 'flick')
This is not really a BC break since the configuration is still read using the old values, it's just updated in the dumped container.  What might be nicer is to alter the code so that `lifetime` is rewritten to `cookie_lifetime` for BC, but that the configfile may also use the updated values as `cookie_lifetime`.  This way there is still no BC break and there is no magic happening in the background which is always going to give someone a headache down the line.
Needs set -eu and copyright notice
We should make a mixin for this.
done

3 ÓÅÎÔÑÂÒÑ 2012 Ç., 2:16 ÐÏÌØÚÏ×ÁÔÅÌØ Sergey Berezhnoy <
notifications@github.com> ÎÁÐÉÓÁÌ:

> In pages/.bem/level.js:
>
> >  for (var n in level) exports[n] = level[n];
> >
> > -exports.techs['bemjson.js'] = './techs/bemjson.js.js';
> > +exports.techs = function() {
> > +
> > +    return extend({}, this.__base() || {}, {
>
> ÏÔ ÐÅÒ×ÏÇÏ ÁÒÇÕÍÅÎÔÁ × ×ÉÄÅ {} ÍÏÖÎÏ ÉÚÂÁ×ÉÔØÓÑ, ÂÕÄÕÔ Í£ÒÖÉÔØÓÑ Ä×Á
> ÏÂßÅËÔÁ: ÂÁÚÏ×ÙÊ Ó ÆÏÌÂÅËÏÍ this.__base() || {} É ÎÏ×ÙÊ ÐÒÏ bemjson.js
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/bem/yac2011-bem-bl-in-use/pull/2/files#r1513837>.
>
>
Ich möchte noch einmal kurz auf die Guidelines verweisen.

- Variablen: _wieHier
- Methoden: BitteSo()
- Bitte kein "var" es sei denn der Klassenname ist sehr lang
- Benutz die regions wie unter http://openfiresource.de/wiki/index.php?title=Coding_Guidelines#Mustervorlage_Dateilayout
- Benutze selbst bei einzeiligen Statements die { und }, also nicht:
if (_Configuration.Tilt)
    builder.Append(TILT);

Sondern
if (_Configuration.Tilt)
{
    builder.Append(TILT);
}

Danke!
`for (x in y)` is unsafe on its own. If `items` is an array, it's better to use a traditional for loop with a counter. If `items` is an object, then you should wrap the body of the loop with:
``` javascript
if (items.hasOwnProperty(idx)) {
  // loop body.
}
```
There are a couple of other instances of that in this file.
No actuall changes
Зачем приводить к нижнему регистру всю строку? Есть метод Character.toLowerCase()
А это корректная ситуация.
Надо просто Arrays.toString() возвращать
Не ругайся.
По этому исключению сложно понять, что именно не так.
Otherwise, looks good, and tests pass.
Ok, I committed the changes and (I think) updated the pull request. Thanks.

-Pete

On Apr 7, 2012, at 8:10 AM, mack-t wrote:

>>         // Important to recognize this async request is doing a get and the user / password is passed in the search url
>>         // TODO: should most likely just use the xhr object and doing basic authentication.
>>         Tomahawk.asyncRequest(search_url, function(xhr) {
>>             var dom_parser = new DOMParser();
>>             xmlDoc = dom_parser.parseFromString(xhr.responseText, "text/xml");
>> -            Tomahawk.log(xhr.responseText);
>> +            //Tomahawk.log(xhr.responseText);
> 
> If you think this should be removed, please delete rather than comment out.
> 
> --- 
> Reply to this email directly or view it on GitHub:
> https://github.com/mack-t/tomahawk-resolvers/pull/1/files#r657866
test_users_account.py:50:26: E225 missing whitespace around operator
I'm guessing this is used for mapping a calendar on the DB? Scrap it and use the Joda DateTime class.
少なくともmayで「はず」はないですね。「かもしれない」「可能性がある」の方が良いです。
The SMS sent should be a confirmation.. so something more like:
  "All messages have been removed."
Duplicate page?  Could it be merged with the other imageJ page?
Тут планировался «финт ушами», но потом я отказался от этого. А `var`’ы остались…
Maybe say that it is only computed if ``compute_importances=True``.
a function defined in the scope of another will also break in parallel
:laughing: :laughing:
Rename to `errors_`?
don't need parenthesis here on left hand side
> Actually I think it would even be more interesting to unroll the 4 variants to remove as many if tests as possible from the inner foor loops:
> 
> 	• sym is True and squared is True
> 
> 	• sym is False and squared is True
> 
> 	• sym is True and squared is False
> 
> 	• sym is False and squred is False

I just tried this out and the results are inconsistent, I don't even see a pattern.

sparse 1000x1000:
(X, Y): 
ratio:  1.08058160443
(X, X): 
ratio:  1.03299965004
dense 1000x100:
(X, Y): 
ratio:  1.1609735269
(X, X):
ratio:  0.89944192903
dense 1000x1000:
(X, Y): 
ratio:  0.986153732902
(X, X):
ratio:  1.07842157842
dense 1000x10000:
(X, Y): 
ratio:  1.03396245798
(X, X):
ratio:  0.97313621599

We would expect the ratio to be always <1. This is a bit strange...
Did you mean `SRCTYPE value` ?
I don't think this should ever happen.
the confusion comes from having two params that take ndk.

-t shouldn't take any param and --ndk value should be used for the ndk path in all cases (normal use and unit tests).

I see no reason for specifying the ndk with the -t option
only read min(length, bufferLength)
Again, consider using `stringByAppendingPathComponent:` here.
As far as I can tell, roles are optional. So this request should check if `$this->role` is not empty and only use it if it's not empty.
Is there an issue if rw_crc is also set?
Synchronization objects have replaced the explicit locks and unlocks.  I have replaced the mutexs with CriticalSections.
Oops, numRows is already taken, how about rowCount() ?
The loop will not be used for retry attempts for inotify.  The call to <code>useINotify</code> will occur only at most 1x per call to <code>monitorDirectory</code>. The call to <code>useINotify</code> will return under 3 scenarios.  

1. Error condition, critical failure resulting in an exception being thrown
2. Returning 0, in the case of timeout.
3. Return 1 if a change had been detected.
`__repr__` needs to be unambiguous.  So this means it probably needs `var_pos` as well.
this exception does not seem to be used in this class
Also test a files that are broken. For example, if a previous run crashed in the middle of appending an event, the tail end of the file would be corrupt.

We should not die on this and, as Mark suggests above, possibly move it into a "dead" (I prefer "abandoned" or "lost") directory.
That sounds possible, but im not sure if the build environment can inherit variables that way. I will test.

On Sep 19, 2012, at 4:13 PM, Tom Prince <notifications@github.com> wrote:

> In master.cfg:
> 
> > @@ -291,9 +291,10 @@ c['builders'].append({
> >      'factory': BuildFactory([
> >          gitCheckout(),
> >          gitDescribe(),
> > +	ShellCommand( name="link iOS libs", description="add sym link to iOS static lib directory", command=['ln', '-s', 'lib', 'build/apple/ios/lib']),
> Can you just make the library location an environment variable, that can be set externally?
> 
> —
> Reply to this email directly or view it on GitHub.
> 
>
Yeah, thats fine. The case it doesn't handle (options.filename exists and is in a subdirectory, but the template referenced in the partial is in the defaults views directory) probably won't happen often or is more properly handled through a ../ relative path. I can see this happening for a header and footer referenced in many templates.
Yeah the only "docs" for whiskers.__express right now are in whiskers.js/test/server/test-express.js.
I'm wondering if we can factorise test-event-core.js and test-event-target.js, as they look 90% the same?
typo: you used `.` instead of `:`
Mention here that our recommendation is to extend Zend\View\Helper\AbstractHelper first. Then we can say that all you actually need to do is to implement the interface.
Mmm but this is a fluid row, so the column widths are percentages. As in just because it fits at one screen size doesn't mean it fits on another. I meant more in terms of the total number of columns in the row adding up to 12 like they're supposed to
this is an element location from where I am picking data. so I think adding comment for each web element is not good.
If I read the code correctly... would it not be the tema's event page and not the library's ?
@shacker is there a reason for putting the `src` in single quotes and then double quotes?
A nice rule of thumb is to use `let` when ever you have the same expression writen twice, like `(sort (rank-frequencies hand))` in this case.
Nitpick: sorted imports?
extra white line here
used in the manage.py
The pkt[1] stuff (host info of the response from recvfrom()) is something I found in other existing modules, I assume it's a convention for trimming up ipv6 addresses. But now that I look at it again I don't use that value anyway, whoops...so it can all be removed. Thanks!
I think would be a better to use cmd_exec if possible
Names should be a little more descriptive
elsif what?
Maybe an extra sentence here to explain that uniform=False gives smaller error overall.
I wouldn't worry about it. It'll be a touch slower perhaps, but there are much less efficient parts of Tastypie than that & it'll be much more resilient for it. Please kill the special-case.
What I meant by `update=1` is just to say that the intent of the PUT is to "do an update". Adding another flag `auto_updated=1` sounds good too, since then it would basically update the auto_updated field of the package in the DB. I realize it's a bit messy, but for proper restfulness I guess you should do a POST with update=1 and then a PUT with auto_update=1.
I know this is trivial but in AMO we use single quotes for defining strings.
1Devs Education Department - 1Schools
Are textures ever deleted?  More generally, who has ownership?  Previously, the caller was always responsible for `destorying` a texture for a material.  We need to keep our story straight here too.
I added some more tests.
Because they could either animate forwards or backwards, so the best choice seems to be to start in the middle.
Yeah, I updated all the code like this to use static clone now.
`item` is not used.
There's no link to this from `index.html`.  Was this used for testing?  Can't we achieve the same with the Sandcastle examples outside of Sandcastle?  We need to make that really clear so folks don't think they need to carry around Sandcastle.
请完善下light的读取
The old code use args[0] where args = Array.prototype.slice.call(arguments, 1). It is actually equivalent to using arguments[1]. That's why the new code uses data here. BTW, I think the else if (isArray(handler)) block will never be hit, because we only accept "function" to be an event listener now (see the "addListener" method).
`#define` needs to be updated.
Yes, I think that makes it clearer. Thank you.
Fixed in next update.
If we used commas, it wouldn't make sense! ;p
I've amended it slightly tho. ;)
"we're not going to redistribute" and publishing it on github seem thoroughly at odds to me.
Questão de nomenclatura aqui: se o calculo eh de semi perimetro, o nome da variavel nao deveria ser perim.
If I remember well, it fails if you don't add it, I think it is used to find assets.
The ^ operator throws an error when testing using ruby 1.9.2. I'd look up the ruby 1.9 equivalent, but I'm about to fly :-).
Though I guess this makes it easier to add the occasional z=0 if we wanted to, so w/e
As a proposal: `TerminalElementOf`?
That was a cute trick. I don't think that I ever would have thought of doing that.
The order should be reversed here. If the leftmost bit is set, it's a signed integer.
It's to avoid using whatever happened to be left on the `front` after whatever used the random number generator last. I can add a comment, but it's the sort of thing that I'd expect a lot of code using random number generators to do, so I don't know if it's necessary or not, though the fact that you asked probably indicates that it is.
Why is j used here?
This could be easily rewritten to not use these additions (which have been the subject of some contention) by adding a case for `T[N]`. Could you please do that so we can make progress with this pull request?
one statement per line
How are the servers quality and the clients quality put into relation? 

Say, e.g. clients accepts text/html;q=0.8, text/plain;q=0.5 and server provides text/html;q=0.5, text/plain;q=0.9 

Will the client or the server "win"? Or should we average the values to weight client and server preferences against each other? If yes, how should we average? Arithmetic or Geometric mean and should the client and server quality preferences be weighted, i.e. to give the client's preferences an advantage?
ehem why we don't add a good mix of backbone to the soup? just saying it is becoming the way to decouple things in js
Almost. What I want is something like this:

    switch($lockToken)
        case NULL: AssertPass
        case String type: AssertPass
        default: AssertFail

I also have an aversion to using framework-specific asserts, given that the concepts of the tests should be applicable for all Azure SDK for *. I'll leave as-in for now.
I will implement it as sample solution in scope of
https://issues.jboss.org/browse/ARQGRA-162
Your xbox-controller wouldn't work very well with the PS3 anyway ..
you should export g
So if a getattribute call requires derivatives, it should call this function three times?

(I see now the renderer side patch -- I was hoping the actual conversion call would be on the OSL side though, so that its guaranteed to always be handled consistently)
* ViewControllerを継承しているクラスは、CameraViewControllerという名前にしましょう
All of these event bindings here should be using `.on()`
I don't think we should be printing "null".  Shouldn't these be blank?
Don't we need an else clause here? Otherwise we'll just swallow this parameter
This fix does not work if the option USERENA_WITHOUT_USERNAMES = True

-- here cant get authenticated user.
Should be removed. Never the case that it's `*Undefined*` - added by nuget package restore target.
@gordonslondon that the point ;) just write the commands the user need to enter to migrate the data!
No need for `test_` prefix. You are already in the test project and test app. :-)
Falta un `-`
``string[]`` ?
Remove @author tags as the author can be found by looking in the Git history
Excellent job on this spec; it looks great.
Don't use `!` unless there's a corresponding non-`!` method that isn't as "dangerous":
http://dablog.rubypal.com/2007/8/15/bang-methods-or-danger-will-rubyist
You cannot change this without writing a migration script.

This will break the code of every single openerp user using this module in production.

I dont see anything wrong with the spelling either according to http://www.merriam-webster.com/dictionary/cancel both spelling are valid. If you are particular about the way it appears, i suggest you use translations.
Should be fixed by  	f00dca5
думаю удобнее проверять безоапсный
Yes, because it is possible to handle errors silent. Tho' I'm not quite sure if I remove that feature...
Nu există deja `jsonify` în flask? http://flask.pocoo.org/docs/api/#returning-json
From one of those online Ph.D. programs.
So... for relative URLs - don't require request in the context.
For absolute URLs - strictly require request in the context, raising an appropriate assertion error if it's not available.
We usually import messages like this:
```python
from horizons.messaging import SettlerUpdate
```

Whitespace around imports logically separates them, so i'd move the storagecomponent line and 26 into the block that starts in 28.
getlocalplayerart() makes the if useless?
use Json::FastWriter().write
add a parent or class
This function could be improved in a couple of ways:
1. Rename it to get_ordered_questions    - at the moment it is misleading as you aren't returning ordered groups, you are in fact returning Question_order objects
2. Actually returning a list of Question object, rather than their intermediate class: Question_order, which should only relly be used internally for ordering, not exposed to people wanting to get the ordered list. You can do this with a simple comprehension:
```python
return [order.question for order in Question_order.objects.filter(questiongroup=self).order_by('order_info')]
```
Borrandolos.
This test case seems odd, or at least at odds with the comment.  Like the other new test cases here, you add N message to a folder and then sync all N messages.  I would this expect to add N messages to a folder and then sync < N messages.

Also, I think there's no real benefit to slinging around as many messages here as the tests are slining, which is a good argument in favor of making DESIRED_MESSAGE_COUNT something that can be externally twiddled like how the constants in syncbase can be twiddled.  The tests could ratchet the desired number of messages down a bit.  Having said that, these tests perform better than the IMAP ones right now, so I don't think it's a problem yet.
I should have tested before making this change, it does not appear to work. It needs to be 
    string.split(/[,|]/)
Why did you inherit from `ActiveRecord::Base` here?
Is this value always required in config?
If yes, the block is fine
Not necessary, could be filtered directly by PHP.
Cookie are visible in phpBB.
As the class is already 600 lines long, move to the **support package** as that one already contains *AbstractXQueryResultMapper*.
Yup, my mistake. Will fix.
I was trying to limit my changes to lines 58-63 to avoid breaking the existing bash building code for native extensions[mac] as per your concerns earlier. However you're right, we should cleanup these commands as well.