labofsahil/github-event-dataset-2012 huggingface dataset:
This is a very naive implementation. I found two things right away, and I won't even begin to talk about performance.

1. It can't find nodes by an attribute value if the attributes are separated by a linefeed (it hard-codes a separator regex of the space and tab characters).
2. It can't find nodes if you don't quote the attribute value exactly the same as it is in the XML.

## Simple test cases:
```ruby
xml = %Q{<root><a i="1"\nj="2">i=1,j=2</a>\n<a i=1 j='3'>i=1,j=3</a></root>}
XMLMotor.get_node_from_content xml, 'a', "j='2'" # => []
XMLMotor.get_node_from_content xml, 'a', 'j="2"' # => []
XMLMotor.get_node_from_content xml, 'a', 'i=1' # => ["i=1,j=3"]
XMLMotor.get_node_from_content xml, 'a', 'i="1"' # => []
XMLMotor.get_node_from_content xml, 'a', "i='1'" # => []
```
Normally, I wouldn't comment like this. But, if you're going to release a gem and then link to it like you did on LinkedIn, you open up yourself for comments and criticism. The proliferation of barely usable gems in the universe is getting annoying.
This comment says "single web role", but it's actually a worker role.
Eeeek! This can't be right.

`getProfileID()` returns a constant value: `"default"`.

This means that each of your `ClientRecord`s has the same GUID.

Don't do that. You should be asking your client delegate to fetch your persisted GUID or generate a new one, remember?
Doesn't look like this revision is actually used by the [download script](https://github.com/ps3mediaserver/ps3mediaserver/blob/7ddf716f41f6d4b8e7dd23e0045fabe46d29918d/contrib/download-pms-binaries-source.sh#L653).
Is there a way to improve that?
Looks good, though I guess this will change a bit when/if you use the runner to launch it.  

Would you move this code to the runner, i.e. is it generally desirable for any Newman app? Or maybe if a command-line option is passed?

Also once the rescue Exception is fixed in the server, should be just a plain old rescue here, obviously.
Actually, I'm probably going to move away from both methods and auto-generate the document thumbnail instead.  But thanks for the contribution anyway.
В Python советуют вместо `if ... == 0:` использовать `if not ...:`.
The function "sonata_block_render" does not exist in SonataAdminBundle:Core:dashboard.html.twig at line 21 
TwigExtension missing ?
You just deleted the code for my comment macros.
Added future support for light level sensors
@geemus  This changed coupled with https://github.com/fog/fog/pull/906/files#L22R81 allows you set implementation-specific defaults for server_attributes. 

We are totally overriding the Fog#credentials here. But since Fog#credentials is really reading from an RC file, we are not overriding the concept.

Maybe Fog#credentials should be changed to Fog#defaults or something similar?
I'll look again, and report back.
Yes, this way it's always set as it was before. This late in the game, i thought it was safest.
We can just put main.js at the same directory of index.html
Forgot to check is_callable here... It changes the API though (was returning false), are we ok with that?
It's actually not a double, but rather a PetscScalar.  I think in sequential.py we should raise an error if PETSc.ScalarType != Mat.dtype.  That way we'll catch errors early.
> s"..."?

You commented earlier: "No need to create a StringContext for a single  
param". Could we agree on a convention when to use which?
See my comments on the other PR
I think you should change, if there is no good reason for using the quotes (we don't need to learn them jvm internals here).
?

just do:  CircuitBreakerClosed.addListener(() ⇒ callback)
Great, Boy Scout!
Make it a one-liner, for teh eyes
Why? isn't this exactly equivalent as just having yourself as the sender of the message?
This implicit conversion is meant to make it easier for people to build their test bundles using the Fluent API in our BundleDescriptorBuilder - with this thing, people can just use the BundleDescriptorBuilder or plain PojoSR BundleDescriptor (whatever they prefer) and mix them up transparently.

I can remove this implicit def obviously, but all it would do is require an extra 'build()' call on a method like this 

  val testBundles: Seq[BundleDescriptor] = Seq(
    bundle(TEST_BUNDLE_NAME).withActivator(classOf[TestActorSystemActivator]))
also, this applies to `pynsq` as well (it's implemented in a very similar way)
Agreed.  The good news is we have at least one test that will fail, so if we fix it, we can remove this and the tests will still pass.
Is it not rudimentary comment?
There's a typo on the docstring (s/GroundShking/GroundShaking/).
Use a HashSet instead loaded with the invalid chars array. You will have one foreach loop iterating on the path and calling Contains on the HashSet.
logic actions should always be got via logic.get_action() as they can be overridden.  Not an issue here as it is a test but good to be aware
as for other comments
Oops. That commented line was so I could see the signature of upload_unit as I was writing this. Forgot to clean that up  :)
If we use `env['PATH_INFO'] =~ @include` then it's fine as long as `@include` is nil, or a regex, or a string. If necessary that could be sanitized in the constructor. This will crunch down all the boolean logic here.
i appreciate your appreciation of snoop dogg :smirk:
You can take a look at https://github.com/xbmc/xbmc/commit/0dddb48fa91f7c292433e055617af7400dd1a0d9#diff-2 for how I did it in JSON-RPC (FileItemHandler.cpp). It's not perfect but a lot better.
Never leave commented code
Missing whitespace between `if` and `(`.
This `include_dirs` block should be defined inside a `direct_dependent_settings` block in the `deps/winpty/winpty.gyp:libwinpty` gyp file.
A comment explaining why?
This can be removed once we have a way to get an `NSAffineTransform`.
The logic here can be reduced given lines 295 and 296
Should be should ;)

Sorry, couldn't resist saying that.
Completed!  Thanks for the tutoring.
For file size optimization and scannability purposes, consider omitting optional tags.

http://google-styleguide.googlecode.com/svn/trunk/htmlcssguide.xml?showone=Optional_tags#Optional_tags
You should disable that ;-)
Yeah, but you should raise it when `#order` is called, not in `#build_order`. So `Post.order(id: :DESC)` should raise an error.
* Fixed font usage is not right.
* s/ruby/Ruby.
* The sentence can do with simpler words - we can just say there are issues and so debugger replaces ruby-debug19 :)
Please, change this to the 1.9 hash syntax.
put a blank line bellow
Technically intercept should return what it intercepted so you can do things like:

intercept[IllegalArgumentException]{ block}.getMessage must be === "OH NO YOU DIDN'T!"
I think that's a typo!
Might want to go LinkedHashSet: http://docs.oracle.com/javase/6/docs/api/java/util/LinkedHashSet.html
I mean not 'return tree' but '{ MissingClassTagError(tree, tagType); tree }' ?
Done. Also evicted a dozen or so other usages of `assert(false)`.
Alrighty then-- game, set, match for the eradication of `andThen`.
Isn't len(qdd) equal to n, which you use elsewhere?
Except it doesn't use parentheses for atom arguments.  So you still have to special case it.  So please add the other printers.  You need pretty print (ascii and unicode, should be the same) and LaTeX (and any others that might be useful).
You probably just want str(self) here. 

FiniteSet already has a str printer defined in printing though. I'm surprised that this is necessary.
Well I established the convention of putting the generator index first. There is no really good reason for it, except that it seemed more natural to me to write `x[0]` and `x[1:]` instead of `x[-1]` and `x[:-1]`. I'm changing the docstring as you suggested.
How come only the first part is 2D pretty printed?
Seems like this is more like a set method than a get method. Or maybe "create_point_at" is a better method name.
«GitHub скопировал проект вашему пользователю»: почему-то у меня пользователь ассоциируется непосредственно с человеком, отчего фраза «ваш пользователь» начинает казаться абсурдной. Может быть, я не прав?
Здесь хочется что-нибудь наподобие «GitHub создал вашу копию проекта» или «GitHub скопировал проект в вашу учётную запись».
I think this should be `scale_dimension` (or `scale_something`) because `scale_limits` can return different things depending on the class
Sorry copy-/paste mistake. I will fix it.
Do these doxygen comments need updating to say they're for the nodal matrix? And does the MELGE comment still apply?
See comments above about nodes/elements.
You think it's better for sending to fail for people overriding the `mailer`?
Maybe it's not very common that this is done though.

Another thing I thought of doing was moving the `$this->mailerCreated` part to a Pimple parameter like `$app['mailer.created']`.
This way (combined with not checking if the mailer service changed) we can allow people to replace `$app['mailer']` and set `$app['mailer.created'] = true;` themselves.
Will do. I had assumed it would be unique, but fair point.
Few typos (dictionary) in the comments
What happens if an earlier end date than the start data is passed in?
What about "Reset Map"?
In general I worry about documenting pre-2.2 behaviors/bugs too thoroughly.
nit: "parent" length
Add a line break before this for clarity.
This PR simply adds a feature to the functional framework allowing you to create and destroy users. If another 'permanent' user is required for some tests, I think this should be handled in a different PR.
The file() method calls the variable() method which returns an empty array if the specified variable does not exist and the default is an array. I do not presume to know the reasoning behind this, you'll have to take it up with whoever wrote that method.
Neither class has one. This is how it is done in UMIL.
Assert that code is 404.
Indentation of this whole method needs to be fixed.
Does not belong in base driver either.
How is one expression related to the previous? And on the first run of this $is_expr doesn't even have a value?
Which is exactly my point. btw the event I ment was renamed and is now called "core.index"
I wanted to use the same variable name everywhere and, to prevent any problems with any existing variables or anything a mod might use, I choose $phpbb_notifications just to be safe.
Please add a comment explaining what happens when topic owner stays at 0.
"Defense in depth". Look it up.
In ele-agent-upgrades the file extension is sha256
Well, from my experience, Hackage was down more often than GitHub so far, so as an intermediate solution I guess it's fine.
still not true as well :-) This work for mirage as the versions in opam-repo-dev are different than the ones in opam-repository: for a dev package, OPAM looks first in opam-repository and it does not find the right version, so it then looks on opam-repo-dev.
In general, it's helpful for readability to take long strings of ```this.that.theother.what``` and save them into a simpler variable name.

```
var whateverThisMeans = patientStore.getData().items[0].getData().patients.length;
for (var j=0; j < whateverThisMeans; j++)
{
  // etc
}
```
tabbing is broken here. Did you run http://jsbeautifier.org/?
As per standard, getMonth() returns month from 0 to 11. (which I missed earlier).
you can check: http://www.w3schools.com/jsref/jsref_getmonth.asp

I have also checked the function my changing my system time.
You can combine getTodayPatient and getSevenDayPatients into one common method "getPatients : function (startDate, endDate)". If preferred you can still keep helper methods, "getTodayPatients" and "getSevenDaysPatients", but these should call the common method and pass in the appropriate date range.
Should the ```package_data``` payload also be placed into the manifest template?
Arguably the current check is better - it's testing the desired behaviour, and could conceivably survive a switch to an alternative immutable type.
A pointer towards ```python setup.py --help``` and ```python setup.py --help-commands``` might be of benefit.
perhaps a second parameter = true would return the default mime type?
modernizr-2.6.2 is out. Worth updating?
I fixed that everywhere.
Should it be T_local_reduce.test_local_reduce_broadcast_some_0 now?
Add a comment warning that stringfunctions.h must be included before jsonfunctions.h 

OR (better yet?) add a proper #ifndef STRINGFUNCTIONS_H_ ... in stringfunctions.h and include stringfunctions.h redundantly, just in case, from jsonfunctions.h
These kinds of changes are preferable to make in separate commits, even if pushed at the same time. That said, I'm guilty of sneaking stuff in as well.
Change the class name in the docstring
I think it's probably fine for now.
Shouldn't we still match and display this error msg in case someone botches the config value?
this is a bit hacky, I know. suggestions? ;)
The file loader does not need to know - Why not have App.cpp be the only
one who needs to know.  File loader finds and loads data from files.
 shallow call-stacks indicate less coupling and are much easier to
comprehend.  The suggestion is not a drastic change - just moving a couple
of lines of code.

On Tue, Oct 2, 2012 at 2:25 PM, Paul Wilson <notifications@github.com>wrote:

> In src/Core/Input/XMLFileLoader.cpp:
>
> > +  XMLQueryEngine xqe(*parser_);
> > +
> > +  int numModels = xqe.nElementsMatchingQuery(query_path);
> > +  for (int i=0; i<numModels; i++) {
> > +    QueryEngine* qe = xqe.queryElement(query_path,i);
> > +    Model::initializeSimulationEntity(type,qe);
> > +  }
> > +}
> > +
> > +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> > +void XMLFileLoader::load_control_parameters() {
> > +  XMLQueryEngine xqe(*parser_);
> > +
> > +  string query = "/*/control";
> > +  QueryEngine* qe = xqe.queryElement(query);
> > +  TI->load_simulation(qe);
>
> The file loaders will absolutely need to know things about Cyclus core! At
> some point they need to report what they have read to something. The
> reverse dependency is important to avoid - the core should not need to know
> anything about the file loaders.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/cyclus/cyclus/pull/382/files#r1742820>.
>
>
I commented under the impression that `effective_default()` may call a potentially expensive (and perhaps even state-changing) callable, which isn't necessary if `include_default` is false. If we're dropping the callable execution, then it does't really matter.
This description is a little misleading, since it doesn't indicate that MTLEqualObjects will short-circuit isEqual: based on an identity check.
Ага, сократил + добавил проверку на то, чтобы это значение не было юнитом или плейсхолдером.
I think that most (all?) strings used as parameters in Lantz are lower case. Also while `mini` and not `min` Is the index of the minimum or the value?
What do you think about folding these into a single `parse(s, T)` generic?
Cool. I'm out of town until Tuesday, but I support both this change (with comment about i <= 1) and re-deprecating isResolved/isRejected. :)
wait, the string has to be the context, sry. I tested wrong.
wouldn't `this.nodeType % 10 === 1` work and be shorter?
Add one parameter to control whether do data sampling here.
Hmm. :) The design document should eventually go into the repository. I would rather have textbook references here (lecture notes are also OK, but 2nd choice since they usually are not as stable a reference).
I think we could write this as:
var i = $("#id_"+prefix+"-TOTAL_FORMS").val() | 0;
Oops, wrong module.  And dalli needs to work independent of Rails so you can't assume ::Rails will exist.
The `caveats` method needs to return a string, not output directly.
I can't stop another maintainer from merging this if they decide it is appropriate, but IMO we have been left holding the bag enough times that we shouldn't make exceptions.
Why is find_domain returning an array?
serait ce args[3] plutot que args ?
que de ligne vide ...
^objects (lowercase)
`'utf-8'`?
I'll call the underling Cython method then. nanops are slower:

In [1]: alphas = Series(np.random.randn(1000))

In [2]: %timeit nanops.nanskew(alphas)
1000 loops, best of 3: 261 us per loop

In [3]: %timeit rolling_skew(alphas, len(alphas))
10000 loops, best of 3: 68.9 us per loop


On Sat, Mar 24, 2012 at 11:36 AM, Wes McKinney <
reply@reply.github.com
> wrote:

> > +        if level is not None:
> > +            return self._agg_by_level('kurt', level=level,
> skipna=skipna)
> > +
> > +        if not skipna:
> > +            if isnull(self).any():
> > +                return np.nan
> > +            ser = self
> > +        else:
> > +            ser = self.dropna()
> > +
> > +        if ser.count() == 0:
> > +            return np.nan
> > +
> > +        from pandas.stats.moments import rolling_kurt
> > +        kurt = rolling_kurt(ser, ser.count(), ser.count())
> > +        return kurt.values[-1]
>
> There is an underlying Cython method you can use (look at
> `pandas/stats/moments.py`). Or port the array math to not use rolling_kurt
> like nanops.nanskew
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/pydata/pandas/pull/932/files#r600214
>
What w really need, here, is a File::Path module, but this change is a reasonable fix for the immediate problem.
This looks like a function that would belong in `r2.lib.utils`
It was a bit arbitrary. I'm not entirely sure how to trigger the execution of the subscript automatically, though. Thoughts on this?
Wondering if we should strip the 'post_' prefix from parameters. It's currently inconsistent between commands:

```
wp post create --post_type=foo
```

vs.

```
wp generate posts --type=foo
```
I'm not sure I was checking translate.document here at all. AFAICT we could just get rid of that check. We could also replace it with:

```javascript
		if(typeof translate._sandboxLocation === "object") {
			var protocol = translate._sandboxLocation.location.protocol,
				host = translate._sandboxLocation.location.host;
```

Ideally, we'd figure out how to check if two window/document objects have the same origin rather than doing this based on URIs, both because it's the right way to do it and because the current approach doesn't work properly if the page the sandbox is created from sets ```document.domain``` and the loaded page doesn't. Then again, the latter case has only happened once AFAIK.
-38
-39
+ DatabaseQuery<T> convert(Query query); ? :)
May need to do `@_batman?.promises` instead, in case someone gets really anxious about calling this before `initializeObject` gets called.
Don't misunderstand, I love `::` for referring to prototype methods, it's just the trailing `::` at the end of an identifier that irks me.
Fixed these three.  I also moved the tooltip so it covers the same DOM nodes as the clickable region, which let me make the code a bit simpler.
Please add curly braces to each block. Also use 4 spaces for indentation.
Comment is too long to put on same line. Move to previous line.
Oh, yeah, I still think this should be shopify side
so the gist is that the old and new styles can't coexist in the same set of options? That seems fine.
Indeed, that's true. I will change that. Thx
oui supprimer la déclaration du provider dans la config pour permettre l'utilisation de plusieurs providers
Have we gained an extra, unnecessary level of indenting here?
`GitHubAccountAuthenticator` sounds reasonable.
I'll add the headers...
??? Don't hard-code URLs?
User such_names_for, no? But why form needs a name? Not ID?
Numbers vs. Strings for these.
Didn't you fix this in Lighthouse #1391?
Yeah, you shouldn't need this (re https://github.com/mozilla/butter/pull/1198/files#L0R200). `mediacontentchanged` will do exactly what you need, afaik.
If this is an asyncTest, stop isn't needed here, right? Should probably move it over.
Can we successfully run closure compiler after this patch?
I wasn't  sure of the best MAX_REASONABLE_TIME - so this choice was based on 32bit int wrapping.
I know you're thinking it should be more specific about what's being rescued - I agree with that principle, but I don't really want to change that behaviour and risk introducing problems.  If we encounter a situation in which this is a problem then we'll fix it and add test coverage.
This is missing a `cwd=` spec at the latest. If we need a git revision number, we should really think about releasing more often instead.
A comment here describing what the job does. Also apache header.
Yeah, you're right.  I didn't really understand Futures when I wrote that.
```js
_path = PATH.join(_path, dir);
```
I agree with ivan. Condition "@run == 'once'" is already in done? So skip? is can be just alias to done?
top! didn't know about validate_hex like like like
ctrl k d!
I completely agree with that. It's better for those who are looking up how to do a particular thing.
I don't like it either. I think a gen_server would be better since there is no longer any blocking gen_server:call to calculate stats, so unlikely to be any large mailbox.

Ideally a process per stat.

This has been tested up to a a few thousand ops a second (each op will call update 2 or 3 times) and seems OK.

To move back to a gen_server is simple, a process per stat for the next release maybe. The amount of load testing I need to do when I change this means a full day to verify that the gen_server approach is better. I went with expediency in this case. Happy to change if there is time.
I find it hard to understand that explanation, can you try to explain what this does?
This really seems like a bug then, as the comment in optionsdialog.cpp reads:

<pre>    // These are shared with core Bitcoin; we want
    // command-line options to override the GUI settings:</pre>
Good coding style IMHO.
Two reasons:

Consistency. This is the only place in the code base that I've seen the DIRECTORY_SEPARATOR assigned to a variable instead of a constant.

Semantic correctness. DIRECTORY_SEPARATOR is a constant and if a shortcut is going to be made it should be of the same type, a constant.

Overall, this change and the required if (!defined('DS')) { define('DS', DIRECTORY_SEPARATOR; } is a net gain of one line of code.
Attention : cet URL... (un ou une URL ?)
Missing space after `+`.
this is wrong when using it with another Doctrine project than the ORM
correct, we get a strict warning otherwise on using [] without initializing.
Hmm. So the native dll would end up next to the assembly and would be loaded by the default interop probing mechanism. This will work if you stick on only delivering an x86 version of Git-Tfs.
On Thu, May 10, 2012 at 11:04 AM, Bussonnier Matthias
<reply@reply.github.com>
wrote:
>> +    };
>> +
>> +    Completer.prototype.finish_completing =function (content) {
>> +        // let's build a function that wrap all that stuff into what is needed
>> +        // for the new completer:
>> +        var matched_text = content.matched_text;
>> +        var matches = content.matches;
>> +
>> +        var cur = this.editor.getCursor();
>> +        var results = CodeMirror.contextHint(this.editor);
>> +
>> +        // append the introspection result, in order, at at the beginning of
>> +        // the table and compute the replacement range from current cursor
>> +        // positon and matched_text length.
>> +        for(var i= matches.length-1; i>=0 ;--i)
>> +        {
>
> really good,
> I just sed spaces after function I think.
> The only problem is with jQuery chain methods where it put everything on a single line. (need to find a good regexp for that)
> I have to try putting // on line end to see if it stops, or try to configure it.
> it also always put dictionnary with one key/pair by line for small dict as function argument is useless/less readable.

OK, sounds like it is helpful though.

>
>
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/ipython/ipython/pull/1711/files#r803734



-- 
Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com
This CSS solution actually solves both problems.
http://imgur.com/dP9s1

The other way to fix it from overflowing would be to add another nested div which can be a huge problem for smaller screens. Less divs, means a cleaner page, less things for the browser to process and easier to make changes in the long run.
Shouldn't the prefix be configurable like for browserchannel? How about inserting options.prefix ?= "/sockjs" on line 17?
The size is inhered from the element it self .. the idea is not to set 
the width / height for the iframe explicitly, and have it be implicit 
per whatever css is inherited.

I would recomend the mediaSpace integration just set the 
'FramesetSupport.PlayerCssProperties' on the target div that they will 
call kWidget against. That should apply the same iOS 3 / 4 css fix to 
the frame.

If we need to support this for legacy reasons we could do that, but I 
would prefer to deprecate 'FramesetSupport.PlayerCssProperties'

--michael

On 05/31/2012 03:10 PM, Ran Yefet wrote:
>> -						$( elm ).height() ? $( elm ).height() : 300;
>> -					
>> -			if( typeof width == 'string' && width.indexOf('%') === -1 ) {
>> -				width = parseInt( width );
>> -			}
>> -			
>> -			if( typeof height == 'string' && height.indexOf('%') === -1 ) {
>> -				height = parseInt( height );
>> -			}
>> -
>> -			var targetCss = {
>> -				'width': width,
>> -				'height': height
>> -			};
>> -			
>> -			var additionalTargetCss = kWidget.getAdditionalTargetCss();
> By removing getAdditionalTargetCss it will break BlackBoard integration.
> Also where do you set the width/height for the iframe?
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/kaltura/mwEmbed/pull/159/files#r911565
ここ、auth_params_for(:foo).uid みたいにすることが無いなら Hash でも良いような。
This is not a string, but rather a number mapping. Later on it does translate to a string, but one that is mandated by rfc5545:
```` ebnf
weekday     = "SU" / "MO" / "TU" / "WE" / "TH" / "FR" / "SA"
````

rfc5545 has stated that [Monday is the default weekday](http://tools.ietf.org/html/rfc5545#section-3.3.10), which is why I set it here. The problem in that line was that I was using 1, which I had mapped to Sunday.

The setting is only important for certain weekday rules, I think Lightning doesn't even set WKST correctly :-)
Dang. That does make it less pretty.
(Review class hierarchy.)
The patch looks good, thanks!  One thing, saving `directives` here is probably unnecessary — if we're parsing a function, then we're no longer inside directives above it, right?
Consistency is good.
this statement is called multiple times -- there is no point to pass params each time. Redesign API or send it only first time.
Since these are options that are only relevant only on the command line (manual intervention) you might not want it in your loadDB.ini file. So, add

```python
    required_config.add_option(
        ...
        exclude_from_print_conf=False,
        exclude_from_dump_conf=False
    )
```
Not this file.  php.ini is under a BSD license
How do you know how many threads is right to run? 

Oftentimes some programs default sensibly by inspecting the number of CPUs. That might be overkill but perhaps we ought to include a verbose comment about a good number to pick. Or at least explain why 10 is chosen default.
Should this really have been included? :)
why making it extend ContainerBuilder ? it would be better to create a ContainerBuilder inside the class instead IMO. This would separate the concerns, and allow you to use the container in a more powerful way by allowing people to cache the container if they want (assuming you provide the needed extension point in the Sculpin or implement it directly)
this looks weird. you don't use the power of the DI component. You are still creating all the object yourself and then setting it. You are using the container like a Zend_Registry...
If we have to do BC I'd vote for pt. 1. IMHO, optional parameters should be after the required ones.
no need for the ``else`` as the ``if`` throws the exception
As far as I know this applies to all predefined classes/interfaces. Perhaps someone could confirm this in relation to Symfony coding standards?

@vicb Can you confirm this?

If so a correction in the docs would be good.
Same code is repeated 3 times in this class, please encapsulate it into method call. Something like

```
[self higlightButton:self.sendLoveButton
     withCompletion:^{
    [self.delegate cell:self didSelectSendLoveToUser:self.user];
}];
```
This will also give that code some meaning - so in future some other coder will clearly understand what it does.
why the triple braces here?
These constants look like they are used in `VoteNotificationEmailJob` but not `VoteConfirmationEmailJob`. Since they're not shared, maybe move them directly into `VoteNotificationEmailJob`?
这个确实是，感谢集鵠。
Can't you remove all those warning flags now that -isystem is working?
This is too close to the `company` method. What does this method provide exactly?
We should not be changing ids of error and log messages between releases
Ok, thanks! I am working on the :reload op for servers and will remove the the unused stuff this branch.

I didn't like basing it on this only, since it would put the server in an out-of-sync state even if the operation was rolled back on the domain. However we do already send a restart-required operation in the domain rollout handling, if servers are allowed to fail in the rollout. This might introduce some other race, so yeah we should talk a bit more about this.
That doesn't work... It tries to add the valve in fact.
I did think the image loading asynchronous issue, but the iframe scroll height does update at this moment. I think the images size are applied synchronously, and expand the space for image asynchronous loading. There may have the chance that we could not apply the image size at first and need to wait until loaded, but this logic works fine for my testing case now. You can comment out line 975 to see the difference. Iframe could not display completely without this resize handler.
directory -> file
nit: see what @lightsofapollo said
This won't work, use `/shared/style/headers.css` instead.
Same comments as above. Also is there a way to factorize some code with setDate?
You could store int in a var 'recentEnabled' or something like that and reuse it later!
if (!this.element.dataset.id)
        return;

and save a level of indent for following code
Cool, more elegant for sure! ;) Thanks!
Please add a comment as a reference to describe what '0x1105' from.
I'll argue that extracting this part would improve readability.
This would only allow you to change content in the files that are marked up using `{curlybraces}` it would make it hard to replace content in third party scripts, for example `http` -> `https`.
`endd` : wink2:
You should check whether `expr` is a `Real` or an `Integer` and then do the base converion. In /every other case/ (not just for a single `Symbol`), `expr` should be returned unchanged. Please add the test case `BaseForm[x + y, 3]` for that.
s/enviroment/environment/
而且。。这个是女主角。。不是大boss。。（虽然最后会演变成大Boss）
If you want :D
It would conflict with the reflection case.  I'm trying to make sure this is higher priority for TupleN.
If I understand your question correctly - we do because we can only merge if both compoundFactorValue and doseFactorValue are present. Let's discuss if this is unclear or I got the wrong end of the stick.
As expected, they do use different HTTP status codes for requests that are unsuccessful, so I replaced the data type checking with code that utilizes those status codes.
Emm, this part's log msg would be: (if no modprobe.conf file found)
```
INFO: SUCCESS
INFO: Check virtio_net module in modprobe conf
INFO: Check virtio module
INFO: SUCCESS
```
A test step without test result and no reason, weird, isn't it?
Well, so we should have some mention of this somewhere.
> I'm not sure how common it is for jsonrpc to be used with multiple endpoints like this, though

gerrit does this... And I agree, I did not like it that much.
yeah sorry, thought I caught that in in time (forgot to push). updated.
No, (for current workflow). If package was in the system repository (cache) downloader will already try to download this package from local storage (because dependency resolver should choose it instead of remote)
I was thinking about it but then I found out that you also have mocked objects in Mock directory. Will change it.
Yeah, it's a bit tricky if the initial bundle can't be grabbed from a secure source. Any way getcomposer.org could perhaps host a secure copy and occasionally grab an updated copy and it's hash for distribution? Single point of failure, yes, but Composer can quickly establish HTTPS to getcomposer.org with an initial PEM from the installer so a secure route is now established? It's convoluted, I know!
Here is a version that checks if composer.phar is installed globally:

```
#!/bin/sh
# Composer Git Checkout Hook

# Process composer.json if one exists.
if [ -f composer.json ]
then
    # Check to see if Composer is installed.
    echo "Processing Composer"

    if [[ ! -f composer.phar && ! `which composer.phar` ]]; then
        curl -s http://getcomposer.org/installer | php
    fi

    [ -f composer.phar ] && php composer.phar install || composer.phar install
fi
```
actually no, it is 2 lines below
Adding these two callbacks crashes when repeatedly calling the `executeAsyncScript()` API call from a language binding. This would manifest itself when running the WebDriver .NET integration tests. Not sure why. Perhaps it's the callback set by `executeAsyncScript()`?
Can't we drop this, rename VideoFileTypeInfo to MediaFileTypeInfo and use this class for all media files?  It's a 1:1 copy of VideoFileTypeInfo, isn't it?
Not really, I think it is up to the project/community to decide if there is any reason to keep comparability with AS 7.0.
Why make this change? Any why roll it into this pull request?

Please provide a separate pull request for this (though I honestly think it's fine as is).
it will be better to call it in_namespace cause if you will pass namespace(not group) if it will return you a projects list
We talked about some changes to the `spd` after you issued this pull req.
You can put the a.password inside the quoteName, it will be parsed correctly,
This would have to be removed if you want this merged at all
`*/utils` are intended for interaction with unwrapped tab / window / ... objects and should be unaware of our custom wrappers. Please remove dependency on this and make sure all the functions operate on raw objects and are not aware of our wrappers.
I'm doing this to avoid extra-long python stack trace, which is useless here.
sensible.  I like the whitelist approach for this.
Good call. I found that convict config layer would give us a default locale if a config was empty, so I think we're good here.
The column "name" has not to be removed
I think this should be a post, not a put. the put method is to store (create or update) a resource. the meaning of post is more flexible, and covers RPC style actions, which reboot seems to fall under, to my mind.
do you see '!'? ;-)
The functionality you're referencing above is from zf *1* - zf2 (a) does not have a wildfire component, and (b) is namespaced. I suspect you are wanting this functionality for zf1; if so, please report it to http://framework.zend.com/issues
Tried changing method signature to render(CheckboxElement $element) but got strict notice: Declaration of Zend\Form\View\Helper\FormCheckbox::render() should be compatible with that of Zend\Form\View\Helper\FormInput::render()
Normally I also don't use "@return void" in constructors, but it is in place everywhere in the framework, so I didn't want to break that "convention".
Actually, for consistency with Logger (which defines writerPlugin()), I guess it makes sense.
Will fix that :)
The only way i can get this to work is to actually clone the ViewHelperManager due to how the renderer is set.
このタイトルすてきです！
Excellent point.  I've added the comment.
DUEL • TodoMVC
`$("#main, footer").hide();` -> `$("#main, #footer").hide();`
you can still set one without the other, but i think it also makes total sense that they be the same name considering the current 1 instance per environment setup
i thought this was still needed, because dart would seach for a packages directory under test/ (or does pub create one there too?)
I should be capable of providing my own instance. What if the listener requires something other, like services?
the opening curly braces should be on the same line as the foreach
Wouldn't it make sense to keep a special atom clause for Keyword here>
There is a problem with this. If the error happens when `Kind` wasn't compiled, this is going to fail. We should probably revert to the previous format. :S
(because we don't care about the concept of generator when using pelican, I guess)
But this is more descriptive?
If we remove this, we should probably remove the context below it as well. 

https://github.com/eric/flipper/blob/b1ca5fe128becf9e3f1f315bc011d9342b509df2/spec/integration_spec.rb#L356

I can do it post-merge though.
missing empty line between the use statement and the class definition
What does fp mean?  File Pointer?
Arguably, "welcome" and "to the Mozilla community directory" as separate lines/headers is just as weird as the second one, especially, doesn't make a lot of sense on its own (it requires context). So I actually think this is more semantic than two separate headers. (If you really disliked the `<br>` + `<small>` combo you could do a span that split them into separate visual lines/sizes, but I think this represents things more semantically than `<span>`s and is perfectly valid HTML.
Should this be @event['check']['subdue'] so it can be configurable in check definitions? why not use has_key? and ! .nil?
Use real password
why init the connection again? sorry if i missed a logical step, the original code is in a mess...
The versioning we do is based on custom mediatypes. It is thus easier in my opinion to have the annotations explicitly on each method, so you dont unintentionally allow some mapping that should not be valid. Of course in simple cases a class-level annotation will suffice.
It is difficult to find a level of detail that is relevant for this small project. I think you idea about having somewhere documenting how to do this, with code examples, is probably just as good.
I'm looking at all this stuff but won't have a commit until later today. This ``Sleep`` may be able to be removed.
Should this also set `engine` to null in a `finally` block? If not, I'm not sure why it checks for null (line 177).
unnecessary hashes make the sky weep tears of angels
AHAHAHAHAHA NO
Explain what NPM is
This thing is cool! Could use some class documentation.
IMO, instead of iterate options, you should update the ``AbstractAsset`` constructor to deal with options.
ardy.... haahha
I'd really prefer keeping the `page_by_destination` method name - it's verbose, but it's a lot clearer what's going on. Trying to sort out the different types of paths was one of my biggest confusions when I started trying to understand the code.
here you need to return the int from the string.
butttt.... you said 2-space indent?
I'm not a dbus expert but this is there a reason why not to use Qt's QtDbus module only instead of mixing two different ways?
workdir gets cleaned up later, but temp does not. We should be cleaning that up to not clutter /tmp.
We should start thinking about splitting this out into various files. Perhaps one for tenant control and one for config control to start off with.
Right, I removed it now.
Discussed this at the stand up and agreed that `canManage` should continue to return a boolean, even if the user doesn't have access to see the content. So there should probably be a separate access check.

Although, `getComments` below would finally perform a check to make sure that the user can access the comment, so this is just more difficult to read rather than being an access bug, I think. Maybe a different combination and order of requests is needed.

1. `getContent`: ensure access and existence of parent content item
2. `_getComment`: ensure existence of content item
3. `if user is creator of the comment, do the delete` (short-circuit)
4. `canManage`, if they can manage, delete the comment.

Then you have an internal function that deletes a comment without access checks that feeds from #3 and #4 .
Would there ever be an instance where the request is successful but the body wouldn't be a string?
I don't see the error... What did I do wrong?
This is how I made it before. In this case full version of command line option is --attack-files instead of --attack-file. I thought it might confuse a user since they might think that they need to specify multiple attack files only (but they can specify one attack file with --attack-files too)

Do you think it worth changing to --attack-files ?
Slechte naam. Beter: 'playerHealtBar`
Why switch this from a version that works with HTML 5 and XHTML, to one that only works with XHTML?
Тут в 25% будет ситауция когда случайным образом выпадет тот же символ днк что и был перед мутацией. Таким образом мутация не произойдет
А что же делать? )
х по заданию - процент. Он не должен быть меньше нуля, но и не должен быть больше 100.
You should merge the following block into the preceding statements. It duplicates effort and it's hard to see how they match up.
This seems brittle. Is there a way to be more certain if you're actually running on a PI? mkdir -p /opt/vc/lib would break the script.
whoops :smile:

I'll fix that.
A subclass of time
this is when the connection is just broken as in a hard failure
@jmikola cool, it should be quite straight forward to determine what can move down into common now there are working ORM and ODM versions.
I don't think we need this method now right?  Can you remove it?
A comment for $columns is missing.
Non related gemekker: maak van die 10000 liever een constante, waarom kun je zelf wel uitvogelen.
>Why did you stop exposing the GitErrorCode as a strongly typed property?

I'm not sure to follow you. `GitErrorCode` has been turned `Internal` more than a year ago (cf 2bd06300e2c8fe02493657aadc1cd114bc44762a)
As above, I'd omit this comment.
ok,
but raw.copy shall remain, right?

(would appreciate it)

On 19.10.2012, at 21:48, Martinj Luessi <notifications@github.com> wrote:

> In mne/fiff/raw.py:
> 
> > @@ -883,6 +884,31 @@ def append(self, raws, preload=None):
> >      def close(self):
> >          [f.close() for f in self.fids]
> >  
> > +    def copy(self, fname=None):
> > +        """ Return copy of Raw instance
> > +        Parameters
> > +        ----------
> > +        fname : str | None
> > +            If data not preloaded in memory a string will open a corresponding
> > +            memory map. else in thie case np.empty will be used.
> > +        """
> > +        if self._preloaded:
> > +            new = deepcopy(self)
> > +        else:
> > +            if isinstance(fname, str):
> Instead of creating a new Raw instance, I think it would be cleaner to use
> 
> new.fids = [open(fname, "rb") for fname in self.info['fnames']]
> ..I think this should work and sorry about the many comment iterations ;)
> 
> —
> Reply to this email directly or view it on GitHub.
>
one option is to decorate the verbose functions.

@verbose
def my_function(..., verbose=None):
    blabla...

and use logger.info as print in every decorated function.

I would have to write a tiny example to see how this can fly.
`ica_raw.info['ch_names'] += [raw.ch_names[k] for k in picks]`
rather than concatenation rewrite this as console.error("Got error:", e.message);
After finishing making the changes please use http://www.jslint.com/ to fix the styles of the code.
Add some javadocs and @Since
"this is the context used when invoking the API function"
Just so it's written down:

We lol'd at rescuing from `ActiveRecord::RecordNotFound` in `ApplicationController` when it should be Mongoid something (DocumentNotFound). Refactor out to a `find_by_username` and raise that error when not found so `@user` can never be nil.
Is it just me or is this indent off by 1 space?
配置中总是包含了，自动的，不需要配置的。
不是
以下配置
I've update the documentation in https://github.com/thoughtbot/paperclip/blob/master/CONTRIBUTING.md. Let me know if that still doesn't help.
In this case one could argue that the only thing that's needed is "withHeaders" and then you can do:

msg.withHeaders(msg.headers ++ newHeaders)

Because otherwise you'll have to add minusHeaders(), etc etc
On Sat, Feb 25, 2012 at 11:42 PM, Aaron Meurer
<reply@reply.github.com>
wrote:
>> @@ -104,7 +104,16 @@ def __init__(self, docstring, config={}):
>>              'Warnings': [],
>>              'References': '',
>>              'Examples': '',
>> -            'index': {}
>> +            'index': {},
>> +            'Usage':[],
>> +            'Note':[],
>> +            'Arguments':[],
>> +            'Strategy':[],
>> +            'Definition':[],
>> +            'Properties':[],
>> +            'Syntax':[],
>> +            'Yields':[],
>> +            'Return':[],

Perhaps, but I really didn't find any heading which couldn't be
replaced by one of the pre-defined headings (in the worst case,
"Notes"). If you think this needs changing, please open an issue (this
comment isn't even displayed on Github, so we're likely to just
completely forget about it).

>
> I agree we do need to make sure that people use "Examples" instead of "Some Examples" and "Parameters" instead of "Arguments" (for example), but a docstring is and should be more than just an API reference.  Limiting to only API reference specific headers either prevents people from writing useful discussions in the docstrings, or else they will end up all chunked together under one section (the simplify() docstring in its present form suffers from this problem).
>
> So I still say we should fix this.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/sympy/sympy/pull/1030/files#r489042



-- 
Vladimir Perić
Нету никакого CheckStateRole http://qt-project.org/doc/qt-4.8/QHeaderView.html
Γι αυτό δεν έχεις κάνει ήδη ένα pull request και το έκανα merge??
In this PR, don't touch to the others sources.

Implement RowAction::setRouteParameters with this condition:
- If a parameter is a string without key, we use the data of the grid else we use the given data.
you should better request ```php $request->request->get('ab_name')```
Please throw a Symfony HttpException forbidden, the message is "Access forbidden"
Is this a reversible migration?
Inheritance can be used here instead of the detailed flag.
ok, will be fixed in the next commit
As for angles, delete one leading space so all kwargs will be aligned.
Why was this added? There are no references to `make_axes_locatable` added.
Yes, when structure will contain text " date " it will get replaced. Even if it would be in value (eg. default value for varchar).
Instead of wrapping the rest in an else statement, invert this if statement and return the `false` array.   Then the rest can be de-nested!
We should probably add `-webkit-border-radius` back in here (and above) since not all of the last 3 major releases of Safari fully support `border-radius` yet. `-moz` is depricated since FF 4.0.
@apex13 Same here, do we need this extra border property?
please move #ifdef to beginning of line (no indention)
What's the intention of removing the title and value of the error here?
That is a good idea. Unfortunately it lands us in a bit of a pickle include wise (with cache.i and btree.i). I'll figure out how to fix it.
Other parts of the file affected too.
Las dependencias de los plugins prefiero tenerlas por plugin. Aún no he encontrado un sistema fácil de instalar las dependencias recursivamente, así que hay que tirar de `npm install jquery` a mano.

Revierte esto.
So >we< now had...
The options are passed into initialize a couple lines later:

```
this.initialize.apply(this, arguments);
```

So the initialization logic would determine what it wanted to do with the options. In the case of wanting to tweak the options returned, you could override `_configure`:

```
_configure : function (options) {
  options = Backbone.View._configure.apply(this, arguments);

  // do stuff
}
```

Maybe configure should be renamed `_configureOptions`.
One more case.
Have you looked at using an SplPriorityQueue for the $priorities?  Would that work?