labofsahil/github-event-dataset-2012 huggingface dataset:
I inferred the single-line purpose from [FormatterInterface::format()](https://github.com/zendframework/zf2/blob/master/library/Zend/Log/Formatter/FormatterInterface.php). I'm not sure where your definition is coming from, but even the [existing documentation](http://framework.zend.com/manual/en/zend.log.formatters.html) alludes to the formatter's job of producing a line of output.

The [XML formatter](https://github.com/zendframework/zf2/blob/master/library/Zend/Log/Formatter/Xml.php) also generates a single line of output single-line if you disregarded the appended `PHP_EOL`, which the Simple formatter also had before I removed it (I left a comment on that change to solicit some feedback, in case it was intentional).

I don't follow what you meant by "consistency with others writers/formatters". I believe the Mongo writer is consistent with the behavior of the existing DB writer.
I think that the return must be verified for avoid return `false`
This is a signature change, and should be removed.
Why make this class service locator aware? If it is to have a factory in the ServiceManager, why not simply inject the response object instead of pulling it from the SM?
I recommend creating a tag in this git repository and then setting this line to point to ' :tag => ' instead of a specific commit.
Also how to run the server and test.
vaild → valid
Can you not assign a formategory to the "Uncategorized" term?
s/Donwload/Download
It's checked for null in the MainViewModel.OnSendMessage, necessary to check for null here then?
@samiluukkonen Mikäs iffi tää on? Mihin sitä tarvitaan?
Those usage of references seems useless.
The closure below will create new variables ```$allAliasHandlers``` and ```$allVariations``` for the scope of the closure.
If you want those to be manipulated by the closure, you must declared them with a ```&``` in the closure definition:

    function () use ( $field, $versionInfo, $variationName, &$allAliasHandlers, &$allVariations, $variationIdentifier )
What's the reason for republishing the events and adding all of the Selection accessor methods to the List class?
On Thursday, 30 August 2012 at 13:07, Leandro López wrote:
> In lib/haproxy/socket_reader.rb:
> > @@ -86,6 +86,7 @@ def send_cmd(cmd, &block) > socket = UNIXSocket.new(@path) > socket.write(cmd + ';') > socket.each do |line| > + next if line.chomp.empty?  
>  
> I'm using 2 spaces for indentation, could you fix this so the code is properly indented?
> I'd like to merge your code, thanks!
>  

Hello,

Sorry about that.. Now it's correct.

Best,

--  
Dominik Zyla
I don't have a link to the Getting Started there yet. Do we even know what the general link will be yet?
I wonder if the best thing to do would be to just pass in method, URL and status and let the exception class construct its own message from that? (i.e. `raise UnexpectedResponse.new(:post, 'http://foo.com/bar', 404)`).

-- 
Regards,
John Wilger

ph: 971-678-0999
http://johnwilger.com
Is this meant to handle the case of trying to link in a native library? Otherwise it should be bitcode and already handled?
why "white" here if we use #fff everywhere else? just curious.
This is going to violate CSP, needs to not be inline.
add whitespace around `"iframe"`
delete this and move this into the asyncTest call as the 2nd parameter
I suspect they're laying the ground for the two tabs setup, which will be implemented with the separation of assignments.

In the mean time, following our principle of "is it better than it was?", I think it's better to have some way of accessing posted claims, which are currently not linked anywhere any more.
yeah, but the the point of this change is windows compatability, right? ;)
and I do have first hand knowledge that the packages will have <code>\\\\</code> in their path name, while the require statements use / as path separator => require won't find the package.
my point was that your latest change removes the 'JSON.stringify' from the name (which basically just adds the quotes) in case it's an unix system. you might want to do something like <code>JSON.stringify( if isWindows then name.replace(/\\\\/g, '/') else name )</code>
generally, I would not break the link here - stylistically, I'd keep this all on one line
While translator updates are no longer pushed out to clients < 2.1.9 (it might be a later version now), translators using Framework have minVersion of 2.1.9
Yeah, I'm all up for changing the wording. Since tags can be large, I guess we can make it very descriptive. Something like "__Zotero could not determine the proper item type for this entry. Please adjust it manually." And also apply it to other items that we don't know what to map them to. Then by default we can map to journal article (at least until document gains more fields) and set this tag.

While this is an automatic tag, I think the idea here is to make the user aware that some items need their attention and in cases like importing from a RIS file, it's probably best that we get their attention right away. That's why I think it's good that this tag would show up i the tag selector and because of the double underscore it should be at the very top. But ultimately, I think it's your call.
Oops, didn’t see you already replied. No leave it like you made it, it's much more readable.
just remove them - nobody like them
hmm. very true.
I haven't tested this, but what about binding to the `change` event? http://api.jquery.com/change/
Never mind, eskil explained this to me. I am just bad at reading.
Missing condition: $criteria->add(AppDocumentPeer::APP_DOC_UID, $fields['INPUT_DOCUMENTS'], Criteria::IN);
Use method "loadCase"

$this->loadCase
Sure does.  Updated.
Since setValue will not bring accurate results, as you described, shouldn't the method been overridden und just throw a exception to ease debugging?
Is this the best way ?
IMO, you can't deal with options in the abstract platform. It is not possible to handle all table options in this way and so, make the method unusable. Options is only here for dealing with custom platform options & so can only be handle in the custom platform classes.
Sure, I'm gonna change this.
I'm not a big fan of this - I get that this was probably rescued because there was no SMTP server in development (Mailcatcher, btw)[http://mailcatcher.me], but if mail doesn't send, I'd like to know about it with a big nasty 500.
My stupid JS-brain always wants to avoid useless closures, but you're right, this would be better ruby.
It comes from http://www.galago-project.org/specs/notification/0.9/x408.html#signal-notification-closed . It can be made an enum even though in the spec they're just magic numbers.
This is an incorrect and dangerous thing to do. Without a transaction here, you could end up with a partially updated model in some cases.
The module still needs to be renamed to oae-config
`return (_isTenantAdmin && tenantAlias === that.tenant)` would avoid having to perform a string comparison for all the non-tenant-admins every time this method is executed.
Yea, the config api does some validation and returns an error object in case some of the parameters are wrong.
This if statement just checks if the return value is an object with a code key (ie: an error object)
I've added a note to explain why it's there.

Maybe the config api should just return undefined in case it doesn't find anything/fails?
What if there are non-boolean settings?

Maybe it's better to use `Setting.unparse`, as it's done in the [sbtintegration.ScalaSbtCompiler](https://github.com/scala-ide/scala-ide/blob/master/org.scala-ide.sdt.core/src/scala/tools/eclipse/buildmanager/sbtintegration/ScalaSbtCompiler.scala)

Also, if this is Sbt specific, consider moving the code in the `sbtintegration` package, maybe where the previous links point to.
> You're doomed then, or you move the collector class inside the method. But I guess that's ugly.

Yes, then I'd rather keep it as it is (but still turning the trait into an abstract class).
Now, I can confirm that it's at Parser.h line #1341 

  /// ParenParseOption - Control what ParseParenExpression will parse.
  enum ParenParseOption {
    SimpleExpr,      // Only parse '(' expression ')'
    CompoundStmt,    // Also allow '(' compound-statement ')'
    CompoundLiteral, // Also allow '(' type-name ')' '{' ... '}'
    CastExpr         // Also allow '(' type-name ')' <anything>
  };
True... I'm now in this habit. :-( Maybe we should consider just copying the mediawiki style for spacing so that we don't have to jump between two coding styles as otherwise I fear this is going to recur as I jump between projects.. !
crap i left that in without turning it into a class -- thanks for catching :D
Yes but I made them separate because I plan on having subconf rules enforced by the gem.
table is missing a $ here. @craigh already noted this earlier.
I was using the variable several times in the page, but I will change it to follow the convention.

On Feb 23, 2012, at 6:00 PM, Steve Johnson wrote:

>> @@ -95,6 +99,9 @@
>> EMR_BACKOFF_MULTIPLIER = 1.5
>> EMR_MAX_TRIES = 20  # this takes about a day before we run out of tries
>> 
>> +S3_COPY_FILES_TO_TMP = 's3_copy_files_to_tmp'
>> +FORCE_CLEAR_OUTPUT_DIR = 'force_clear_output_dir'
>> +
> 
> These aren't really necessary. The convention in the rest of the code is to not store every option name in a constant.
> 
> --- 
> Reply to this email directly or view it on GitHub:
> https://github.com/Yelp/mrjob/pull/378/files#r483540
Remove please :-)
I didn't know that this was the syntax that clang uses (I'm still trying to figure out where they documented it). If I can find that page, I might convert more of Dscanner's output to the clang format.
Интересное решение у вас, необычное. Но самое главное что работает действительно правильно.
Ну, тут основной момент в том, что если создается пустой массив, то initWithCapacity не нужен, достаточно простого init'а. А в данном случае, поскольку переменная не инициализирована - в ней может быть _любое_ значение на момент использования, например какой-нибудь очень большое. И initWithCapacity: может отъесть много памяти, в общем-то ему не нужной.
Именно так, честно говоря думал прокатит :)
k

"Some browsers like Opera will ask the user about increasing the quota when it exceeds a set threshold"
Reviewer says: "I agree, your change #2 is a real improvement.  However, could you be more explicit?
Should this actually be a throw? This seems like a pretty critical error.

If not, what does this.log return? Since the normal codepath seems to return the builder, it seems weird/dangerous to deviate.
Ah, no, we don't. `self` will be some temporary class that conforms to `<RACSubscribable>`.
great catch!!

On 20.12.2012, at 06:09, Eric89GXL <notifications@github.com> wrote:

> In mne/epochs.py:
> 
> > @@ -767,6 +775,9 @@ def save(self, fname):
> >          data *= decal[None, :, None]
> >  
> >          write_float_matrix(fid, FIFF.FIFF_EPOCH, data)
> > +
> > +        # undo modifications to data
> > +        data /= decal[None, :, None]
> By nasty, I mean it took me about an hour to track it down :)
> 
> —
> Reply to this email directly or view it on GitHub.
>
please set the statuscode of the response in the case of an error to 500 (internal server error)
I don't like changing the returned shape of things, but we could do that. Do you think that's more useful than having it default to an 'id' field?
I mean conceptService.getAllConcepts returns a list, atleast you need to test that you are getting back the correct count by looking at the standardTestDataset.xml file
Why not the following?

```scala
val server = new NettyServer(…)
Runtime.getRuntime.addShutdownHook(new Thread {
  override def run() {
    server.stop()
  }
})
Some(server)
```
What's wrong with calling `&method`?
This zstyle is also no longer valid.
I prefer a `.. versionadded: 2.1` block over this, you should place this block before this list.
How about adding a link to the routing chapter?
使用 satisfy指令就能同时通过IP地址和密码来限制访问。
И это как-то странно.
Порядок инклюдов + лишняя пустая строка + нахрена тебе тут IJH?
Not sure if ```align(4)``` is correct, but without it the assert fails with ```(8 != 4)```
Finally, some activity!

The main difference between these two functions is how the queries are done - logActions() combines all the data into one insertion point.

Having as little code duplication as possible is a very good and desirable goal. While replying, I thought of a way to solve this while keeping compatibility - logAction() becomes a deprecated pointer to its bigger and badder brother. What do you think?
The factorial of 0 is 1
Better question...what was the use of it?
check this date formats, it would be better to support timezone
consistency : idTask should be named taskId
how are these cassettes created?
Ok, I'll add static. I'll need to check in the standard to clarify my knowledge about this later.
can you add `db_info.id` instead of `context.tenant`. That would be more useful here and being in the description we can pass that on to the backend systems.
Just nitpicking here, but I prefer avoiding the braces for if (trace) log.tracef(...) statements. Even if it requires more line wrapping...
@danberindei Interoperability with embedded mode is something I'll be tackling for [next major version](https://issues.jboss.org/browse/ISPN-2281).
Missing the testName attribute.
Copied from somewhere else. The point here is to override the super test and don't do anything. This line could be removed.
I think the callback is called on the same thread, so you don't really need an atomic variable here.
I can easily clean this up, no problem.
I have no problem with updating CHANGELOG, although it is getting sparse to do so now.  But this definitely should go into whats_new.rst.
magic number. Perhaps use the length of the last dimension.
:fire: nice hotfix patch!
s/testsetup/self.testsetup
90% of the time you're right, but I don't think we should exclude the 10% time when it's reasonable. There are many, many cases where it's more valuable to use a stale value (presumably while another process is regenerating the proper value), rather than enter a stampede (aka the dog pile affect) or pause to wait for the value to return.

Keep in mind this isn't saying get has to return anything- it doesn't. What this is saying is that the option is there for a library to expand it's functionality within the scope of this proposal. If a library wants to have a "standard" Pool and Item class, but also wants an extended Item class that handles validation differently (such as by doing any of the following- http://stash.tedivm.com/Invalidation.html ) then this standard should not get in their way.
I personally think most of this spec is asinine, and doesn't do much to actually help us do what is most important. His comment on having a standard Package manager is one of those things.

For the record, we use 2 spaces at work for everything except Python.
Instead of selecting the children, could we use the `s3d-search-result` class to select all elements?
@dooremont sorry I overlooked these
We should, I've created a separate ticket to take care of that. SAKIII-5319
Footer should probably no longer be here. I'll fix.
ahhh, you are right, I thought we had a helper method on the org for that
Hmph. I see what you mean now, will definitely have to think about fputcsv for the next iteration - managed to fix the character encoding issues for XML, but not CSV.
```set_experment_created_at``` is basically the init method on the db side though (see ```Experiment::Base#save```, which is also called in ```ABTest#save```).

Also in terms of unexpected side effects, isn't creating a new experiment itself already an unexpected side effect..? (by which I mean I don't think setting created_at is really an unexpected side effect since all objects in the db should have a created_at the moment they are created)
Lint doesn't like this line at all, WT_MIN is taking a bunch of unsigned things as args hitting it with a sign and then assigning it to a signed variable?   I'd suggest expanding this line, and making added an unsigned variable, just for clarity.
Why `PHP` but not `UUID`? `PHPUUIDGenerator` might look a little weird though.
Didn't test this yet, really. Actually this feature has been on the back burner. But I'll do some work on this after a while. Thanks for the tips.
Ok, going to change that for now. We might want to change that later though, as we might want a mutable list for easier editing of documents.
The assignment to ``$options`` seems to be dead. Are you sure it is necessary, or maybe you have a typo somewhere? Please also check line 166.
You could implement this without `every?` using `filter`:
~~~clojure
(and (not-empty a-seq)
     (pred (first (filter pred a-seq))))
~~~
Probably not a big deal, but I disagree, @realityking I think it's almost a defacto standard, at this point, that the image resizing process also create the subfolder, if that location does not exist.  

If the calling routine wants to create it first, nothing is stopping it, but causing an error when it doesn't exist seems like an unfriendly way to deal when a folder can simply be created and the application continue on it's merry way.
Did you mean

    $extension = $input->getCmd('extension');
This lets me do default arguments and not have to specify them every time. Plus I don't see that Runnable takes any args.
*da potrdiš* -> *potrdi*.
Can you make all those buttons not be duplicated? So now we have buttons twice in the templates. Extract this to one template which you then include on both places.
Hm, then maybe "Omreženi uporabniki"? And we also change English to "Networked Users"? If this is our tagline, we could use it ...
Are you ok with exception here :)?

    venue.li.effect(
        'highlight',
        {
        },
        slow
    );
See in the code when request is not passed in. Because sometimes we could maybe rather fail then return unrestricted access.

(Or return simply empty list if request is not available.)

(So the difference here is between deny by default or allow by default.)
Rename this to `HoroscopeProviderBase`.
`test` -> `dependency`.
My editor automatically strips trailing whitespace, so this is just reflecting that. Same below.
I can't tell, is this a tab?
I am not too keen on having this `filter` parameter. It adds extra complexity in Poltergeist for not too much gain I think. It's pretty trivial for the user to do:

``` ruby
page.driver.network_traffic.select { |item| item =~ /some_url/ }
```

in Ruby.
makes sense. 
so if you want to get field called 'X' five levels down, you should give all its parents right? (say A, B, C, D).
Please document this.
Nit - import Constants so this all fits on one line.
do you think it makes sense to put all constants in a separate module, and to import the module in any other module that references the constants?
Thanks for making this more flexible.
No space inside around arguments.
This won't get called for `close` session_events by the default dispatch logic in the `ApplicationWorker` base class. It will call `close_session` instead (the close event is different because you can't reply).
for error throwing in line 176, 244 can be separate, but I usually don`t work work with git do the rebase (I usually work with mercurial)
Why not change to @Inject annotations?  The change is almost there =D
Just a note for the future: You should ignore change of this file by doing :

git update-index --assume-unchanged Resources/archipel-commit.js
Not a huge deal, but in general, I think it's better to keep formulae like this all on the same line, so I prefer the previous line breaks.
有个终止符，嘿嘿
hashPart is empty on index URL. For example:
"articles/" would not trigger the code inside the if.
qmake has nothing to do with make.
qmake if the makefile generator of Qt.
qmake is provided by the Qt installation packages. If Qt is not installed, compiling oqamldebug is fails.
So effectively I need qmake as configuration script.
Yep, I'm going to point out style inconsistencies, some places are significantly more strict about it than I am. Consistent style leads to much more readable code which increases maintainability in general. Fix it or not, if I see inconsistent style I'll be changing it to match. For the same reason I correct extra line space, extra spaces on the ends of lines, etc. It is all technical debt that has to be paid down eventually.
I should be more explicity -- I will hold off pulling this until you add some safeties.
Maybe say "custom metadata"? There are other things in that dict that are used, like content-type, links, indexes, etc.
c'est ce que j'avais en tete oui
We adhere wherever possible to PEP8, which states:

> Separate top-level function and class definitions with two blank lines.

See [PEP8 blank lines](http://www.python.org/dev/peps/pep-0008/#blank-lines), our [styleguide](https://wiki.mozilla.org/QA/Execution/Web_Testing/Docs/Automation/StyleGuide), and a useful [precommit hook for checking PEP8 conformance](https://wiki.mozilla.org/QA/Execution/Web_Testing/Automation/PEP8_Pre_Commit_Hook).
So, it's better $header-color than $white?
I'm not convinced this causes disk IO, it returns a `java.io.file` object, which according to documentation is just an abstract representation of a file. The next line however `file.exists()` _will_ cause IO.

This was obviously a generic approach to the problem, I am open to suggestions / PRs.
Still want these?
would that really be an improvement for performance/security/ease of coding?
unwanted spaces
Oops, your correct. I have not used this in a while.

Still, having an function name like this much clearer then makeDisposition.
And considering fabpot's note about overwriting the parents function.
Ah, OK. No, keeping it as the upstream looks is the right choice. Sorry for the noise!
Let's just cast it to a string internally to be nice.
Sorry... sending again...
Если произойдёт исключение в конструкторе BufferedReader, то FileReader останется открытым.
Этот код несколько раз повторяется. Его надо выносить в отдельный метод.
Это ограничение я ввожу чтобы избежать переполнения стека, если мы будем сериализовывать объект, который где-то в глубине содержит ссылку на себя. Ведь мы же не можем сериализовать такой объект.
getSimpleName returns the 'Simple Name' of a spell. E.G Confringo's Simple Name is Fireball (at least I think this is what evilmidget is trying to achieve?)
I'm afraid this is a carry over from my Haskell days (Haskell compilers warn about unused variables unless preceded [or named] underscore)
why not getting it from the form ? It should be marked as readonly when the field is disabled
You can then access the @errors@ property to determine if validation failed.  については、

@errors@プロパティを参照することで、検証が失敗したかどうかを確かめられます。じゃないかな。
thanks。それにします。
Budget_total is all money needed for a project including other sources. money_asked is the part that's been asked from this website.
Can we remove ``imp`` and ``functools`` from the main ``skimage`` namespace (or import it as ``_imp``, ``_functools``)?
Also rename ``equalize`` to ``equalize_hist``?
Sorry,  just figured out that for some reason pip8 got turned off in my IDE. Now I can see what you mean, I'll fix it right away.
either "a sparse Bernoulli random matrix" or "sparse Bernoulli random matrices".
sweet! I was working on putting this into the common tests but progress kind of stalled. Can you do ``SVC`` next ;)
The variable aliasing could be improved here. `mp_length` is weird.
yeah, i think you're right that it should die and just say something like 'destination repo not found' or whatever happens whenever you try to push to a repository that doesn't exist because, at least from the end user's perspective, the review being created is sort of secondary to the dominant 'hg push' mindset.

it's also likely that whenever you call `hg.outgoing(ui, repo, dest, opts)`, you'll probably catch that error.
The value in this feature is in removing noise from JSON output.  Being able to select which noise to remove seems a bit overkill for the use case.
throw an exception
fix order: Container, Display, DisplayManager, qColor (no need for the qtgui part)
The log is below, on both failure and success.
Wondering if the isDictionary could be inside isWorthAssigningDirectly...
If `SDKROOT` is not set, then it defaults to an empty string and `/usr/include` is used. This is to support Xcode-only installs. I know that this change will probably not go upstream since the base/Makefile is multi platform.
Are you.... kidding me ?
I removed yesterday the `_commands` dict following your advice... :(
There's an issue here, as Augeas doesn't reload files that were saved earlier in need_to_run with the "newfile" mode as it thinks they've already been saved.  I'm going to file a bug against Augeas, but I think the way is should be designed to work is aug_load->mode newfile->save->mode overwrite->load->save.  In which case, you need to swap aug.load and the set_augeas_save_mode lines so it'll work in the future.

For now you can force all the previously saved files to reload by doing this before aug.load:

    @aug.match("/augeas/events/saved").each do |file|
      @aug.rm("/augeas#{@aug.get(file)}/mtime")
    end
I'm not convinced that we need dimensions in the popover. Artists would probably be good though (since on complete collection and folly pages the artist isn't evident, and if it's a collaboration then it's also not evident)
Add messages on the assert. Example. Assert.assertNotNull(dashboardPage, "Dashboard Page was null")
Couple of newlines would not hurt.
Good catch on the unintentional aliasing since it can behave differently in 1.8 and 1.9.
83719 : Basilic diff.php file Parameter Arbitrary Command Execution
elsif res and res.body =~ /names.nsf\?Login/

Don't think body needs .to_s
cmd.exe /c del? Or you don't have to do that in any scenario?
Lots of dependence on readline here
s/append/prepend/ ?
I am guessing you are trying to follow [this mixin example](http://strd6.com/2010/10/javascript-mixins/), which is a bit confusing in terms of naming. It was 
```JavaScript
function Guy(I) {
  I = I || {};
```
on that page. I don't think the author encouraged the argument names starting with upper case letter regardless of the context. It was just because lower case "i" is often use to refer to index or iterator, and the author wanted to use upper case "I" to mean "myself". I am more comfortable to make class names starting with upper case letters, and object names starting with lower case letters. So I am fine with both of the following code snippets:
```JavaScript
function Bindable(B) {
    var bus = B() || EventBus();
```
or
```JavaScript
function Bindable(bus) {
  bus = bus || EventBus();
```
Don't bother adding dman for now. Just add catdoc, detab, dget, tolf, findtags.

I know expand exists, but it doesn't on Windows, which is why we have detab here.
Need to list Mobile Web along with Android
Hmm... Yes, I can see how all of the precedence stuff makes this rather confusing. Let me think about this one.
That's a good compromise. Done.
Commented out code makes me nervous. Can you remove this? See also the line at the end of this function.
NIT: superfluous blank line
не надо. охотно верю что логики тут нет
Shouldn't this just be:

```
def self.from_page_part(part)
  self.new(:fallback_html => part.body, :title_symbol => part.title.to_s.gsub(/\ /, '').underscore.to_sym)
end
```
OK thanks for trying. Generally often the bitwise version may be faster because it has less jumps and the branches get executed in parallel (gcc does the replacement transparently sometimes).
not bad !
We would like to keep the extra extension to make it easy to track where is generated code coming from (foo.ext1.ext2 is generated from foo.ext1), can we undo this line change, or is was this causing something to breaking somewhere else?
똑바로 하삼
You do not need a break if you are doing a redirect.
Should the L of Licensed indented with the res tof the text?
brackets only when needed.
Should try-catch this, and fail (abort the request) if the write fails (e.g. buffer exhausted)
Was in kinda crazy dev mode for a demo/end of sprint since that last email.
Will have time tomorrow to rock this out though.

- Andrew

On Thu, Sep 20, 2012 at 3:47 PM, Mike Willbanks <notifications@github.com>wrote:

> In src/ZfcTwitterBootstrap/Form/View/Helper/FormElementWrapper.php:
>
> > @@ -264,6 +264,11 @@ public function render(ElementInterface $element, $groupWrapper = null, $control
> >              $html .= $escapeHelper($label);
> >              $html .= $labelHelper->closeTag();
> >          }
> > +
> > +        $elementHelper->getView()->plugin('form_radio')->setLabelAttributes(array(
>
> Any updates ;)
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/mwillbanks/ZfcTwitterBootstrap/pull/7/files#r1655479>.
>
>
I know I suggested to allow this on methods, but I'm not so sure anymore. 

Basically, we always exclude them unless you have set a @VirtualProperty annotation. So, adding @VirtualProperty, and then @Exclude does not seem to make sense to me. So, I'd say keep this annotation as is.
fair point, let's leave it as is :)
I'm going to remove the Traversable handling, because `YamlSerializationVisitor::75` uses `array_keys`. On a Traversable object it fails.
There is already a strem publisher registrar for the test class. You should not create another one.
your author tag
Yeah - although this removed a number of lower level IFs, Sonar still reports it as a 13.
I'll add it but this is a moot argument. Yes the assertion was not there . . . not they one you proposing and not the one i have now and that is why it was not bought originally.
Passthrough is not needed for this project. (This is to follow which users listen do data, like to have a real-time list of connected users.)
Very odd! But yeah, definitely still there!
Spacing consistency. I'm not sure what @addyosmani prefers, but it should be consistent.
Fixed in follow up commit. However, looks like the prettifier doesn't deal well with the text if I don't add the comment indicator `//` in front of it, it searches for keywords and highlights them. I've left those for now.
What's the coding style for lambda functions?

Is the following OK?

```php
$myClosure = function($someArument) {
    // some statements here
}
```
this is good :)
Sure. I'll take a look at this tonight, if I get a chance.
This is wrong.  You don't need to use a ModelForm for this search form - we don't need a model at all.  

You need to rewrite this to use a normal form where you declare the form fields.  You should delete the models.py file below.
moved the newCorpus function out of the CorpusEditView and the CorpusReadView to reduce code duplication.
query string gives you 100% control over what is passed to the RDFE endpoint to filter logs as an OData query. The --top, --skip, and --type provide a convenient ways to specify most common ways of querying logs for the rest of us.
I don't think this is going to work, if your JSON object has spaces in it, which it probably does this will never be parable, you need to use Array.slice to combine everything into a single string again
Good for the narrative. For the docstring I would put:

While `crammer_singer` is interesting from an theoretical perspective 
as it is consistent it is seldom used in practice and rarely leads to
better accuracy and is more expensive to compute.
Instead of checking can you just do `liClasses.push( this.className );` and just let it inherit everything?
I could have just added `rescue nil` to the end of this line, but I didn't want to silently disregard other unexpected exceptions, so I went with a more verbose approach.
That does not answer my question though. 
Btw, IndexingActionType.UNCHANGED is a adjective describing a state whereas the other values are verbs. I think this should be consistent.
Will this actually raise an error, or will ENV['name'] just return nil? If the former, this begin/rescue block should be split into two - one to test for name, and one to test for grabbing the object from the DB.
The description for both `read` and `write` are wrong.. The methods take arguments according to our Stream specification here:
https://wiki.appcelerator.org/display/guides/Stream+Spec
"If the passed mimeType is null, the mimeType will be determined using the file name."
amen brother. subz rulez.
The conf dir can be located using win32-dir gem which defines the Dir::CommonAppData constant, which does the right thing on 2003 and 2008. Facter already has this dependency, so it should be straightforward to add
One can also treat the stages as a queue, and pop as you go. That'll eliminate some memory usage and the counter, and totally prevent multiple uses.
It is, Was just gits diff noticing a formatting change.
If you updated the below code with my suggestion, this would become.

    images = {
        lang: []
    }
-1 evals to false so you can just say
if (frameworkModules.indexOf(pluginName + ".js"))

don't need change it though just a thought.
Need to add a lot of comments here, this is a big confusing method. Please follow the rdoc doc pattern and at least address the params that you expect callers may specify.
There is no reason for this alias at this point. From what I've could understand in IRC:

It looks like the controller cannot be found on @Flavius system without a Di alias. He will open a bug report about it.
Yes, our bundle switching class (BundleManagementServiceImpl) calls this...