labofsahil/github-event-dataset-2012 huggingface dataset:
Dropped the TODO, too much hassle.
`el` is leaking globally here. That should be avoided. Have you run `grunt` on the code here?
`element` is being redeclared, drop the `var` statement.
The prefix `_` is useful for organization purposes. Yesterday, for example, I need to look for all private names in one of my applications in order to perform little modifications on them. So I look for the pattern /_\w*/.
@etiennesegonzac @mounirlamouri This is not a 'blocker' for the review, but sometimes I dont know the 'limits' of launching an activity and I would like to know when I should/should not use it. In this case, this action is executed by the 'User' when clicking the notification, so  for me should be an activity.... Why are not using an activity (before we were using as an activity)? Which is the main reason for avoiding activities here? And furthermore, Does it mean that we should not to add an 'Activity' to 'Notification' in this type of scenarios? Why? Thanks all!
Can we try to do this w/o images?

```css
  border: solid 0.1rem rgba(255, 255, 255, 0.2);
  border-right-color: rgba(0, 0, 0, 0.2);
  border-bottom-color: rgba(0, 0, 0, 0.2);
```
nit: please use either `apnPrefNames` or `APNPrefNames`
you're right, I didn't notice the others :D
so we can leave it like this for now, but someday we should change all the color names for real
On 14/06/2012 20:12, Paul ADENOT wrote:
>> @@ -0,0 +1,337 @@
>> +/*jshint curly:true, latedef:true, undef:true, browser:true, devel:true, loopfunc:true */
> I'm willing to release this code under whatever license you find convenient (MPL should work, but WTF works for me as well). The above link is correct, but I do not plan to update ogg.js in the future, so, as said in an other comment, you guys are the upstream. It was really a one time effort to parse ogg metadata for gaia, originally.

<3
Probably we could put directly 'document.querySelector('#icon-edit').classList.toggle('disabled');' instance of two methods
idem for closing bracket
This could use some refactoring along my suggested lines above, too.
(let ((project (edts-project-file-project (or file-name (buffer-file-name)))

Also, for some reason it seems to be a convention in emacs to separate file-name
adding some unit tests for this it's broken in a few situations
Should be changed once app manager is merged in.
Filter out bad characters before this call.
``field_missing_inventory_line``
Well, I thought the whole reason we were looking for alternatives to "scratch" was because sometimes the source-target relationship does not apply (like in the hotel example).  In the general case we just have two variables which are intended to hold the same value, but there's some funny business going on in between them.  So there's not a "real" variable and a "scratch" variable, just two different versions of the same value.  That's what I was trying to convey by "translation".

On Jul 13, 2012, at 3:57 PM, John Freeman wrote:

>> @@ -0,0 +1,61 @@
>> +(function () {
>> +
>> +  var proxies = hd.__private.proxies;
>> +  var factory = hd.__private.factory;
>> +
>> +  var Translation = function Translation(source) {
> 
> I don't like the name "translation" because that implies (to me at least) that the scratch is a target rather than a source.
> 
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/HotDrink/hotdrink/pull/41/files#r1162241
Just wanted to play it safe in here (looks like a lot of JParameter has moved in a form or another into JRegistry). I guess you're right on these, we could get rid of few version checks...

I didn't know that you're following my commits. :)
Reading this comment reminds me of the ``SourceStamp`` parameter of the ``Trigger`` step (http://buildbot.net/buildbot/docs/current/manual/cfg-buildsteps.html#triggering-schedulers)(https://github.com/buildbot/buildbot/blob/master/master/buildbot/steps/trigger.py) Maybe some logic could be shared there ...
yep. it's much more performant on big strings.
I thought I had answered to this, but I don't see the comment now. Weird.

Either a define or an exponential increase would be fine with me. A config option would, I think, expose internal details to the user the he shouldn't need to worry about.

If you like the define better, please use something along the lines of:

```c
#ifndef LISTVAL_INCREASE
# define LISTVAL_INCREASE 1024
#endif
```

This way you can configure with

```sh
./configure CFLAGS="-DLISTVAL_INCREASE=102400"
```

to increase this value without having to change code.

Best regards,
—octo
When you download package there is one way to do it : 
- zip|tar|whateverarchive = copy the file
- git = use git clone
- hg = use hg clone
- etc.

When you want to archive a package, you have the choice between using the vcs command if available (git/hg archive), or standard method (zip or tar a directory or files). VCS archivers are optionals, there are just here to "optimize" the archiving method depending on the source type.

Btw, instead of creating the phar file with `buildFromDirectory()`, I could use the Symfony Finder to ignore VCS files.
@stof the "I mean this is the description of composer update not of composer require package/package" was to be taken in context as "I mean this _looks_like_ the description of composer update not of composer require package/package [so something must be wrong here or the description is not very clear]". @igorw did a correction that changed the description so anyway I am still waiting or will try to work on the test to make sense of this. Thanks for clarification. Yeah you are fast but there is always the warning nothing is perfect :)
Typo here: `$` should be `%`
https://gist.github.com/4264179.git

これでいかがでしょう？
ternary operator + spaces around operators
なぜピンクが一番最後なのか？
最後は紫か緑ときまっているでしょう？
Same is in [earlier comment about heritage](https://github.com/mozilla/addon-sdk/pull/484/files#r1569286)
why set expires in the future? We can just set the cookie, if it can be read, `enabled = true`, and then set expires to `-1` (or anything older than right now).
Space after `if`. I also don't like C-style one-liners, as I love consistency in code layout.
I'm more than happy to change this, but it would be really nice to have some tangible framework for ranking / evaluating stylistic choices like this in the future.  I often tend towards being verbose and explicit because I feel like it makes the intent more clear, even if it comes at the cost of a few extra lines of code.  (This is more of a general comment about style-related changes than anything about this one in particular.)
this line isn't needed, see loop above
This isn't going to work since it only gets checked when the gem is built. The resulting gem should work on both 1.8 and 1.9.
I think it's a special case, the other option would be the reverse while as explained here https://github.com/wet-boew/wet-boew/wiki/Optimize-your-JavaScript,-CSS-and-HTML-code#wiki-JavaScript_code_optimization
I don't think this does what you think it does. The pre-condition is that the `$name` does not exist in `$this->data`, or that the value is null or an empty string; when we get to this line, if we `allowEmpty()` and the value is required, then we're good.

I think you're likely struggling with file uploads, and we have a fix in the develop branch for that. If not, I'd need to see the specifics of your use case, because as written here, it's not only a BC break, but also intentionally breaks a specific feature we coded in the implementation.
missing global namespace '\Exception'. However in the unit context you could write $this->fail('message') instead
We can't change this behavior. You should revert this change.
add @author and since to all functions.
n'importe quel*le*
please add a comment with explanation and try to prepare a small example to send to developers
Warum genau wurde da die description aus den Settings wieder entfernt?
Can you resolve this TODO?
Change this same way as others (first compare, then then db call)
CommandException prints an error message to the player
-> `updateList`
No inline code please
Early return can be applied here to reduce one indentation level.

```php
foreach (...) {
    if ( ! (is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value)))) {
        continue;
    }

    // ...
}
```
shouldn't this be a delegate?
Not sure if this is a style choice or not, but generally we don't quote object keys:

```stacktrace = {
  frames: [{ ... }]
}
```
use app_class instead of app_cls? cls is short for "clear screen" or something...
Likewise, can you send these changes the way of the archetypes as well?
Seems like in this case, we don't need to cloneNode, nor do we need to change the volume? Why do we even need the Audio element we create in Mix_QuickLoad_RAW?
i tried =)
I see, it's down there, I got confused you did indeed change that so this should return an array, my bad.
Assuming I'm reading the `template.render` code correctly, the `FACEBOOK_CLICK_GOAL` here should be `goal`.
You can remove this. Also, single-quotes for a docstring? What are they smoking ;)
On 19 November 2012 14:04, Austin King <notifications@github.com> wrote:
>
> In lib/i18n.js:
>
> > @@ -99,19 +99,22 @@ exports.abide = function (options) {
> >          lang = 'db-LB'; // What?
> > http://www.youtube.com/watch?v=rJLnGjhPT1Q
> >      }
> >
> > -    resp.local('lang', lang);
> > +    set_local = function (k, v) { resp.locals[k] = v;}; // Express 3.0
> > +    if (resp.local && typeof resp.locals === "function") { // Express
> > 2.0 Support
>
> Should this be && typeof resp.locals !== "function"

to be fair I haven't tested it, I browsed around on a test to detect express2.0.
I can asses it works for 3.x

--
Niv Sardi
I'm borrowing the idea from the "Spacing" section on this page: http://www.kongregate.com/pages/css-lego-library

Do you think it's enough to spell out the naming convention in LESS/CSS comments, or would you want the class name expanded?
store a reference to wrappers.length
Another idea for this issue would be to unwatch the page after the asserts are done.
We had a similar test in the AMO project where we restored the initial user's settings after the test was completed using a try - finally block.
https://github.com/mozilla/Addon-Tests/blob/master/tests/desktop/test_users_account.py#L163
I worked on it a little in my local version, i thought of loadWithEvents() which shows explicitly that Events are loaded along with calendars which is not the case with a simple load() method. What do you thinks of it ?
As I understand it, Rackspace isn't like EC2 - it returns a randomly generated password from the Fog call to create a new server. We need that to set up the SSH access. I'll check with Darksiez tomorrow though.
Copy paste error in comment? (See next few methods as well)
I know that we've been discussing this Jaromir, but as I see it now, it seems much clearer to not to duplicate the variables here. If you are in the _forms.scss and you want to figure out where the variable comes from, it is always coming either from ```_forms.scss``` or ```_vars.scs```. (it should never happen to be coming from other place if we code our styles corectly)  And it is kind of pointless to mention the variables here just to note that they are used in this file...
What is the reasoning for this? If I'm reading right, it says that min === target === max, so why call this in the first place?
nit: extra spaces in this line are extra.
Looks like you end up using `y1 + diff` everywhere. I think it would make more sense if you passed `y2`.
`LazyCachedNode` is now Serializable, in case somebody sets up a workspace cache as replicated or distributed.

Not sure that's really going to be useful, but making this serializable is all that it should take to make that possible. After all, while `WorkspaceCache` contains a `Cache<NodeKey,CachedNode>`, the actual values should be `LazyCachedNode` instances and will never be `SessionNode` (which represents the Session-scoped delta for a given node).
Doesnt matter, the file as created doesnt have one.
Should be new5-1
In my opinion, the only good optimization here is to replace absolute value (10) by counted width of "#widthcontrol-handler". Other than that - we need some means to give space for that very handler, and I'd rather not using some fake transparent divs for that. Rough JS is better in my opinion. Anybody else has an opinion?
Not sure you want to log this in std logs, as it is not an error / wrong / exceptional condition.
Use ezdebugsettings to log this?
tout не удачное название --- кажется, что это время выхода, а это не так
vector<char>
Removed the new one - see below.
is `jQuery( "<div>" )[0]` smaller?
`retain` does currently work, it gives the ability to manually control the lifetime of an object, should it be needed.

Based on commit 16ce9f6f44a8da66de9bbf18dada4a158a7d6317, I noticed that the trampoline proc doesn't need to be retained. The trampoline proc was originally retained because RM was deallocating it, but that was when BridgeSupport was being used to call `class_replaceMethod()`. Now that we're native code, the retain appears to be unnecessary.
You'll want to add the pages that got passed in to this.
@bebef1987 you should also use `s` rather than `self.selenium`, please fix wherever you see the latter.
src/openslide-vendor-leica.c:412:28: warning: unused variable 'mainImageIFDTo' [-Wunused-variable]
    src/openslide-vendor-leica.c:412:6: warning: unused variable 'mainImageIFDFrom' [-Wunused-variable]
- Delete top level comment, no reply
- Delete top level comment with reply
- Delete reply, no reply

These three are already in there, I'll add in the rest.
Might be better to maintain 'getter' naming, so `getStaticBatch`
I would rewrite it as

```scala
thread.frames.asScala.zip(stackFrames).foreach {
  case (jdiStackFrame, scalaStackFrame) => scalaStackFrame.rebind(jdiStackFrame)
}
```
I don't think you need 2 separate DriverChain. This could be directly in the DriverChain (it is fully BC and does not force to add only advanced drivers in it)
Sorry, I completely neglected to review this file
don't stick _t on the end of variable names. _t is usually used to indicate a type.
Have you actually observed a case where $_GET works over $_SERVER['QUERY_STRING'] ?

I'm not really seeing the point of making this an additional method of path determination.
Same here and anywhere else.
Should this be here? I would prefer it to be configured in the php.ini file, or respect the config file. This being here seems a little odd.
все так - хорошая идея - взяла себе на заметку
Отсутствует проверка заменяющего символа. По условию он должен отличаться от заменяемого.
Move the check for valid lights into perform_wear() as well.
Assuming generators could be changed to work lazily (see my note below), I think `+[RACScheduler deferredScheduler]` would be a better default.
Should assert that `block` isn't `nil`. `-selectMany:` was doing that for us before.
Use the same docstring as in decompose_raw for start stop
these should be debug otherwise it clutters too much the shell when verbose=True
Yes it works too, but I don't know why because neither ConceptSaveHandler nor ConceptNameSaveHandler invoke the setters.
and no merging of different arrays together, so using it should be a last resort. This also means it should bnot be listed first here
This does not belong here IMO as only the ROLE_VOTER retrieves roles. Other voters may vote on some other criteria. For instance, the ``AuthenticatedVoter`` will vote based on the class of the token
Should we add a note here that, although this documentation uses constants for configuration, the user can store his configuration *anywhere*? Or is this obvious enough?
Ah, thanks for pointing that out. To be fixed of course.
Same here for translations
please capitalize the first letter of each class

also, one docstring example for each class.
I think this needs an error check.
This is a great start at a guide.
Remove the unwanted "with" please.
А вот перед этим ньюлайн бы не помешал.
Скобки лишние обрамляющие.
可以不要 hidden 就不要 hidden
This is for users with large amounts of data that want to use infinite scrolling while at the same time breaking the data up into smaller queries
I've been a bit busy these days (and Spuds is on hiatus), I'm trying to catch up now, sorry...

> 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?

Yep, that would be fine!
because the string 'MAXRECSDONE'  is a internal value specific to this task, like 'BAD' or 'WAIT' 
changing it in only one place (forgetting the 'case' on the returned value) caused a bug because the constant self::STATE_MAXRECSDONE is NOT the same value...
changing those internal string to parent const will come later.
That is a good idea. I will be away until September and will try to modify it as soon as I can.
@accessd сделал :)
I changed these because I was messed up my CURL command and wanted to know for sure which one I was getting. :)
Would it be better to use item.strip() since it gets rid of all whitespace(not just one whitespace character).  we could use strip like this:

list = items.split(',')
trimmed_list = [ item.strip() for item in list ]
shall I wait for removal, or do you want me to merge this one first?
you can comment either on the class javadoc or comments in the code directly. Not in github as it will be hard to find them later when someone else tries to understand it to extend the functionality
Why do we need another public method? Clients can just call getMemoryUsage() and get the same thing.
I also just noticed there are spaces after each `forRefSpec:`. Can you please remove those?
I understand that *actual* timing is a bit crude so if there is a better solution, it would be good to update this test. In the mean time, it kind of does the job.
This will need removing. It was a flag to show that the command ```set_ticks_position``` wasn't necessary before.
This is called a list, not an Array in python, unless there are some semantics I am unaware of
when I tested it on my phone it was working...
Is this even necessary to have if all that tearDown is doing is calling to the superclass? Wouldn't the tearDown in superclass be executed instead if this wasn't defined here?
You could also call a function in Component.onCompleted.
I added a note in the header to clarify what implementor means, but I
don't quite agree that your revised wording is more clear so I will
leave it as is if that's alright.
Good question, I am open for suggestions ;-)

I would say header names as keys for sure. The third one is the most correct one i guess.
>cringe< when seeing hard coded resolutions. Nooooooooo. :)
@gsilver 2 spaces after the `:`
We can also replace the 2 `10px` with one
Pq as views tem dados "cravados" ? Imaginei que vc escreveria uma classe responsável pela coleta dos dados, via requisições HTTP, e as fixtures seriam utilizadas para mockar os testes desta.
Minor point: All form elements should be contained in a form, which at least supports the basic feature on submission. In this case, it would mean redirecting to a URL with the correct preview GET params set, which in turn means toggling the initial layout based on those GET params (and ideally history.js support). Not required for now, just thought its worth pointing out, since its a basic CMS design principle.
IE8'll fail at this line. 
http://kangax.github.com/es5-compat-table/
Inderdaad. Wat denk je van "URL-naam" of iets dergelijks?
That's true for a relative URL, but in this case $plugins_base is a fully qualified URL from elgg_get_site_url()

so the link ended up looking like <a href="http://localhost/http://localhost/elgg/plugins/developer/admin">

I don't think that's an Elgg bug is it?
Are you sure the method ``attach`` can be called on ``$emSharedEventManager`` (of type "object<Zend\EventManager\SharedEventManagerInterface>|Boolean")?
Is it possible to add a ``@param`` annotation for parameter ``$url`` which defines a more specific range of types; something like ``string|array``, or ``null|MyObject``?
You are totally right.
Reverted back the change.
It was done only for me to be able to see test behavior on simulator and there is no need to make this change at all.
I'd prefer it if logging was per-class:

private static Logger log = Logger.getLogger(OSSocksServer.class.getName());
This can be simplified to 
````ruby
3.upto(6) { |i| @bud.circuits.create(channel: i) }
not to be all grammar authoritarian, but 'media' is also plural:)
Actually why is it a POJO?
Changing avatars, switching between facebbok image and avatar.
Anyway, I think we could do this better. Dependencies should be resolved in JavaScript on the client. So user should get feedback there, dynamically.

On the server I would just test if everything is correct and display an error. Error should be possible in practice only if somebody is doing something nasty. In reality JavaScript would take care that everything is correct. But we just check for every case.

So this your approach to require user to submit the form to verify it is so 90'.
In case we add updates for deleted/changed/updated posts. We could use new type for that but I thought action is better. Don't know. :)
OK, but I also need porvider name - not only url:

<a href="http://findyyy.com/bla/bla/sss.xml">findyourfate.com</a>?
Hmmm good question. I don't even know where or why I came up with verification. I will change verywhere to validation.
Then add '' to choices???
Why `{}` default? It should be `''`. Empty string.
It is not a good idea to define global variables. JavaScript shares all global variables all around. A good practice is to keep them really to minimum or really just to those you have to use to communicate between different scripts.

I would put both those variables and function definitions at the end into `$(document).ready({ ... })` scope.
Don't know exactly, I think with *the*, but do however you want. We will give this later on to somebody to check. ;-) For now just to have the content we would like to have, this we can improve later.
OK, add link to that pull request. Or just do it like it is written there.
Did that now. Working.
Do we need to keep the old create_slug implementation?

Your unidecode based version looks superior to me.
This regex should be built from the months hash. Also, this hardcoded hash should be replaced with `I18n.t('date.month_names', :locale => :de)`...

And then this whole method should be rewritten so that it works with all languages - like the following:

```ruby
def translate_date(date_string)
  # No need to translate if locale is already en-US 
  return date_string if I18n.locale == :"en-US"

  english_months = I18n.t('date.month_names', :locale => :"en-US").compact
  locale_months = I18n.t('date.month_names').compact
  # Create hash of locale month => english month
  months = Hash[*locale_months.zip(english_months).flatten]
  # Create regex for locale months   ( /Jänner|Februar|.../ )
  month_regex = Regexp.new(months.keys.join("|"))
  
  # Finally, replace locale months with english months
  date_string.gsub(month_regex) { |match| months[match] }
end

```
this class is about transforming data so imo it shouldn't talk about form options and just say the array format is wrong
Had fun yesterday night, some beers with Chris Harris a 10gen engineer. He also suggests to not change the default and convinved me of his arguments: since there is no "safer" option, just different options which are correct depending on the use case, we really don't know and should not suggest anything different.

The default of the driver is "well known" by users, it would only be confusing if the default would be different when using Hibernate.
My use case for this is we prefix every file with a server variable, so it would go something like:

req1[TAB]
require_once '/path/to/file[TAB]';
require_once $_SERVER['PATH_PREFIX] . '/path/to/file';

I have a local snippet set up to insert the prefix too, but that's only relavent to our code base, so I save lots of key presses by combining two snippets.
This actually causes Sinatra to 500 on this action.
Nevermind. I was looking at an old master. Looks good to me!
Hrm, did you test this? This command should be surrounded in double quotes so `$WORK_DIR` is expanded..
what if not use_systemd?
This is a bit a strange function. Seems also to require that the first item of the nested list case is actually "td" or "th"?
The extra one space indentation are funky.
It should not be called "service". We have changed that for the network logger too recently, a "service" is really only a scope service, like EcmascriptDebugger or the like.
Inline function, should now have `{` on the same line. Only old code though.
There's not much more to skip?
Quoting Robert Norris (2012-08-18 20:23:21)
> In src/LuaPushPull.h:
> > +inline void lua_push(lua_State * l, double value) { lua_pushnumber(l,
> value); }
> > +inline void lua_push(lua_State * l, const char * value) { lua_pushstring(l,
> value); }
> > +inline void lua_push(lua_State * l, const std::string &amp; value)
> { lua_pushstring(l, value.c_str()); }
> > +inline void lua_push(lua_State * l, LuaTable value) {
> > +     assert(value.GetLua() == l);
> > +     lua_pushvalue(l, value.GetIndex());
> > +}
> > +template <class T> void lua_push(lua_State * l, T* value) {
> > +     assert(l == Pi::luaManager->GetLuaState());
> > +     if (value)
> > +             LuaObject<T>::PushToLua(value);
> > +     else
> > +             lua_pushnil(l);
> > +}
> > +
> > +inline void lua_pull(lua_State * l, int index, bool &amp; out) { out =
> lua_toboolean(l, index); }
> Is there a reason you used output args instead of just returning the value?
> Actually now I look at it, why is any of this necessary? The templated ones
> perhaps, but for the basic types? Or is it just to try and make a standard call
> that works for all types?

The output args and the standard call are all to circumvent some
inconveniences in the template system. I needed a standard call to use
in my template methods since the C++ template model doesn't allow for
template functions/methods to be specialized.

The output arg is because otherwise every lush_pull would have the same
signature. I am far from being a fan of output arguments, but in that
specific case they were the only solution I could think of.
offset[0..2] does not seem to be used
looks like pinv_wexog is not used if ``isrestricted``, move into an if?

also this part and rest might be better in fit than in initialize (similar to GLS) ?
On Sat, Sep 8, 2012 at 9:39 PM, Josef Perktold <notifications@github.com>wrote:

> In statsmodels/discrete/l1_slsqp.py:
>
> > +        x = results
> > +    ## Trim the small params
> > +    trimmed = [False] * K
> > +    # Don't bother triming the dummy variables 'u'
> > +    if trim_tol == 'auto':
> > +        fprime = score(np.array(x[:K]))
> > +        for i in xrange(K):
> > +            if alpha[i] != 0:
> > +                # TODO Magic number !!
> > +                magic_tol = 0.03
> > +                if alpha[i] - abs(fprime[i]) > magic_tol:
> > +                    x[i] = 0.0
> > +                    trimmed[i] = True
> > +                # If fprime is too big, then we didn't converge properly
> > +                # and we shouldn't trust the automatic trimming
> > +                elif alpha[i] - abs(fprime[i]) < -magic_tol:
>
> This condition looks to tough especially if it raises an exception.
>
> For now, to be able to look at the results, I converted this to a print in
> my version:
>
> elif alpha[i] - abs(fprime[i]) < -magic_tol:
>                     #raise Exception(
>                     print(
>                         "Unable to trim params automatically with "\
>                         "this low optimization accuracy")
>                     print i, alpha[i], fprime[i], magic_tol
>
>  —
> Reply to this email directly or view it on GitHub<https://github.com/statsmodels/statsmodels/pull/430/files#r1561340>.
>
>

Agreed.  Josef and I are going back and forth on the google group about
this.  We're trying to avoid having this condition happen except in truly
pathological cases.  We're trying different solvers now.
Should this not be `context "Exceptional"`?
Let's discuss calling this a builder and the pattern here.
Good catch, PMD finds this as well.  I found this solution:

    private static final ThreadLocal<SimpleDateFormat> XML_DATE_FORMAT = new ThreadLocal<SimpleDateFormat>() {
        @Override
        protected SimpleDateFormat initialValue() {
            return new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ", Locale.getDefault());
        }
    };
We had separate actions here to distinguish banners in the sidebar from those on the top of the site to apply different filters and styles. I believe, using one action is cleaner solution, but we need some kind of `type` field then. What do you think?
+1 for the extra filter
So you plan to rekelvinate by completely synchronously?   I guess that's fine, but I'd assumed rekelvinating is just resubmitting a job to the queue--or am I missing that code somewhere?
On OS X, this failed for me when I was not added to /etc/sudoers.
It failed with:
```
    /tmp/brooklyn-1335968098758-VF9zvNpU.sh: line 17: syntax error near unexpected token `(
```
which is the line:
```
    (test $UID -eq 0 && (rabbitmqctl -q stop > /dev/null 2>&1 || true) || sudo -E -S (rabbitmqctl -q stop > /dev/null 2>&1 || true) < /dev/null)
```

I am re-testing with sudoer rights...
I'm never sure about the expectedExceptions stuff. Very useful sometimes, but it also would cause the test to pass spuriously if there was, for example, an NPE during the setup part of the test. 

On balance I think it's only a good pattern if all of the setup is done in the \@Before methods. I therefore prefer:
    try {
        new JmxmpClient().connectTls("service:jmx:jmxmp://almacretin.local:11099", clientKeystore, "", clientTruststore); 	 184	
        fail();
    } catch (Exception e) {
        LOG.debug("Test passes with expected exception", e);
        // success
    }

But no strong feelings.
Agree not a big deal to stop merging...
1. This line of code to be misaligned for a long time (until someone happens to fix it)
2. A separate pull request / commit to just re-align it (so unnecessary clutter in `git log`)
3. Making the change here and squashing with the other commit so that the change never even appears in people's history

This pull request in particular isn't blocking anything else because it's just removing unused code, so less rush for it compared to your average pull request.

---
I was waiting to see if Alex had any strong opinion before merging...
I'll ping Alex to check.
@octatone Firefox <= 2.0.0.20:

```js
typeof /1/;
// => function
/1/("abc123");
// => ["1"]
Function.prototype.call.call(/1/, "abc123");
// => TypeError: Function.prototype.call called on incompatible /1/
Function.prototype.apply.call(/1/, "abc123");
// => TypeError: Function.prototype.apply called on incompatible /1/
You're right! I missed this when I converted the sample code to be simpler (i.e. not use a separate class). I'll fix it. :D
Hi Michael,

If you are connecting to the same Site, you need to include all cookies to
keep the session open.

... It should't be activated to an external connection.....

In the original "Include Component Plugin", it's used to load a component.


Regards,
Anibal
I've also fixed this one as part of PR 196 - there was also a bug with showing the name, as the log line didn't take care of the end (with db_end).
こうした方が良いとか、あれこれ。
Shouldn't we use an existing library like https://github.com/broofa/node-mime for mime types? For example, this function suffers from the trailing slash (eg if I ask index.html/ then I get the source code of the file as the content type is the default text/plain).
These are the same as Ruby's. Awesome!
I checked my .gitignore but .project is in there. Not sure why it's showing.
Yeah, that's not a fix at all. If they don't stay asleep, that must mean the Life() proc isn't called properly on them.
As long as telemetry is not up, we need to have separate methods of
detection - USB pid/vid for instance...
On Dec 10, 2012 11:38 PM, "peabody124" <notifications@github.com> wrote:

> In ground/openpilotgcs/src/plugins/coreplugin/boardmanager.cpp:
>
> > +        if (!list.contains(vid))
> > +        list.append(vid);
> > +    }
> > +
> > +    return list;
> > +}
> > +
> > +/**
> > + * @brief BoardManager::getKnownBoard
> > + * Returns a known board for a given vendorID and productID
> > + *
> > + * @param vendorID USB vendor ID
> > + * @param productID USB product ID
> > + * @return A board type if the board is known, NULL otherwise
> > + */
> > +IBoardType* BoardManager::getKnownBoard(int vendorID, int productID)
>
> I haven't read it all but this doesn't seem right. We shouldn't rely on
> VID/PID to identify a board type - that should come from FirmwareIAP
> BoardType / Revision.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/PhoenixPilot/PhoenixPilot/pull/79/files#r2374905>.
>
>
Ok, should I place that on the info group or nowhere at all?
Same here. Use ARM_CM4F instead.
Could you add a comment here for which timers/functions require two irqs?
In this case since I'm the only author I'm transferring the copyright entirely to the phoenixpilot project.
Note: don't delete the old methods, just replace them - we don't want to break things unnecessarily.
Also do you know that all the ILog providers live in https://github.com/ServiceStack/ServiceStack.Logging ?
And it doesnt look like we really need to make the distinction - everything is a string in yaml, no?
@yuhunglin sorry for asking but since I won't be in the office today, can you merge and add the comment yourself?
We should declare this element (By.TAG_NAME, 'body') at the beginning.
[`assert_select` accepts a block](https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/testing/assertions/selector.rb#L113), may make this a bit more clear?
Is this code safe in case the user sets sparse to `True` and the above would set it to `False`?
Can you please make this protected? :)
Yes, my plan is to make this PSR-2.  For now, I just copy-pasted what I had previously in the spaces_og patch and renamed where appropriate.
But it's not useful for code reading.
You make this test case unreadable for humans, as contrary to making the error messages readable for humans.

I can agree that the cost to improve test case is expensive. But it is necessary cost.
We should *always* prefer code readability than ease of writing. It is a principle for the good code.
The yml document has been updated.
The UI.SIZE and UI.FILL has not been implemented yet. At this point, I'm not sure what will be supported as far as UI.SIZE and UI.FILL.
When adding new params to a function signature, it's a better practice to add it at the end, so it doesn't break existing calls that might expect the params in a certain order (In this case log to be the argument following blackberryNdk)
not used in this file, put in the .cpp. Same with `using namespace v8;`
I just realized this won't be true if the file does not exist. Use fail() instead, which also checks the badbit.
platforms is duplication for that element
Yes, but I don't think that's what metavar is for. I could be wrong. Either way it doesn't seem to affect the functionality but I wanted to bring it up.
Sorry for the confusion, by post a link here i meant here in github
traduire device par appareil ?
c'est joli
-s ?
This should be:
@Option(name = "-s", aliases = "--service", description = "List service offerings")
Sorry, this is the root device, not the additional EBS I was thinking about. I'm used to fog's defaults for these tutorials :)
I think the extra space should be inside the group, otherwise it will be expecting 2 spaces together: (u\d+ )?
What's the aim of this change?
I agree --numRows looks wrong.
since this is a guard condition it would arguably be clearer without the else.
Changed method signature to return void.
Should I list all of the target processes for a user to select? If there are more than target processes (for example, mythor1, mythor2, ...), what if a user selects mythor1, but the log is under /var/log/HPCCSystems/mythor2/ ?
Nope, it had 4 tests that executed perfectly. And passed... ;)
Should be called getOperatorAsText() or something more general
Have you confirmed that they jhtree tests actually get included? My guess is that if there are no calls to jhtree, the static link will be removed.
I'll amend myself in this case.
Why "Zend" here?
The explicit setting is not added in the PR and regardless of that it'd be nice to assert in this test that default blob endpoint is generated correctly.
This will bubble up as an error message. It's inline with what we do in other places.
MS is the SI standard approved abbreviation for "millisecond", and that kind of stuff is allowed in camelcase. If we were spelling it out, it would be connectionTimeoutInMilliseconds, which seems pretty long for little value.
Personally I prefer not to use the trailing "if"/"unless" on a long line, since it's easy to miss. But rather than hold back the pull request I can just edit it back again next time I'm in the file, Style Wars style.
Hi Manfred - the unit test failure is now fixed in my master branch. I 
am looking at the other problem, I know what it is and the simple fix is 
to add an empty 'res' folder to the project. I think a better fix would 
be desirable so I am looking into that now.

cheers,
Andi

On 16/10/2012 05:26, Manfred Moser wrote:
>
> In 
> src/test/java/com/jayway/maven/plugins/android/phase05compile/MakefileHelperTest.java:
>
> > +    {
> > +        // Split 'foo/bar/baz' (note no leading slash)
> > +        String[] expected = {"foo", "bar", "baz"};
> > +        assertEquals( Arrays.asList( expected ),
> > +                      MakefileHelper.splitPath( new File( FOLDER_FOOBARBAZ ) ) );
> > +    }
> > +
> > +    @Test
> > +    public void testSplitAbsoluteFolderFromCWD() throws IOException
> > +    {
> > +        // Split absolute path of 'foo/bar/baz' (note no leading slash)
> > +        List<String> expected = getCWDSplit();
> > +        expected.add( "foo" );
> > +        expected.add( "bar" );
> > +        assertEquals( expected,
> > +                      MakefileHelper.splitPath( new File( FOLDER_FOOBAR ).getAbsoluteFile() ) );
>
> This test fails for me on MacOSX. Can you confirm that it works for 
> you on some unix system?
>
> —
> Reply to this email directly or view it on GitHub 
> <https://github.com/jayway/maven-android-plugin/pull/134/files#r1850525>.
>
I would prefer to let users install extra dependencies as they enable checks
If the ntds.dit path contains a space, this command will break. Best to surround in quotes.
This would be more useful if it checks the login credentials.
Interpolation is a better way to deal with this kind of idiom.
```
  data = "cmd=#{datastore['OMP_cmd']}&text=#{datastore['OMP_text']}&login=#{user}&password=#{pass}"
```
OK, I understand the issue with multispectral images, but it souds like we're a bit overtuning the parameters: I think we should keep the API as simple as possible. Can't the issue of different ranges be addressed beforehand by a preprocessing function (for example in the color module?)? We could mention in the documentation that we expect each channel to have the same range. 

Also, what for do you need the depth parameter? Could this feature be addressed by some preprocessing instead?
Yes: `preferred_plugins` is just the list of plugin names, so the PIL plugin get checked for availability just like normal.  (I've verified this behaves properly when I disable the PIL plugin.)

This is, however, a hack since I'm modifying the original list (as I note above, 'imread' is the last function in the list to prevent side effects).
By moving to use the parse_list function mentioned about I think I might be able to remove this code. I added it in because import_suggestions was returning a list of lists.
ok .. tweaked it some more in 90e585b25f3e470e9354ca0aafd0390a9d82e858
I would put this at the end of the preceding line.
If the language is not valid then your translation file is not defined.


For this reazon best first thing is check if the $lang is valid and then check if the translation file is found file_exists($file_dict).

That way if the language is not valid (does not exist in the `$valid_langs`) the condition no longer evaluates whether the file exists.

Is more economical. Check if a element exists in an array that check if a file exists
!didScale
That is incredibly stupid. Did you file a bug with node?
Added link to the iOS guidelines.
Not sure about "gone wrong" but it's a known timing edge case.  I have added a warning log message and comment
Correct. I could remove extra "if" statement but then the diff wouldn't be that obvious.
You have two spaces before "event'.
`_autoCompleteString` was previously being leaked
use IPA header from setup repository
I would probably use the later, but could support either way depending on what felt right as you got into it.
> as readable as possible as plain text

This wording is a little off.
should be ``array|\Traversable``. Iterable does not exist in PHP
Yeah, I guess I got a bit carried away there. Been looking at too much Rails code lately :)
Actually, I thought more about this, and this works.  Here is the code path:

if [self data] is nil and newData is nil, [newData mutableCopy] returns nil and [self data] stays nil.  No problem, the next call to appendData with a non-nil newData will replace [self data].
if [self data] is non-nil, everything works.
In the beginning of the function, you can have something like:

```
callbacks.ready = callbacks.ready || function() {};
...
```

That way you don't have to check everywhere.
This is just bad considering that this package only works with express `2.x`, this would currently install the latest `3.x` version.
Ah, my last experience with Symfony was around the time that CodeIgniter was launched (5 years ago now, I think?) and I swear they used to use YAML for their configs.

Anyway, if you have any suggestions or bug reports, don't hesitate to let me know... only prerequisite is that we are only implementing things the Ruby parser does.
duplication with the one in settingcontrollers.py --> Add a utility function to a common place
Indent by an extra level.
It consumes the range if it's an input range rather than a forward range, but that's normal. I believe that even std.conv.to will do that, so I see no problem with the name. "To" implies a conversion, but it doesn't say anything about what happens to the original. Personally, I'd probably have picked something more like `genCRC32`, but `rangeToCRC32` works just fine.
Special-casing `allSatisfy` for functions would only make `allSatisfy` work with functions. It wouldn't fix anything else. It just so happens that the example uses `allSatisfy`, but that's just the example. `Filter` uses it in _its_ example, and any one of the functions in here which operates on `TypeTuple`s could be used in the same manner with `Lambda` but would have to be special-cased otherwise.

It seems to me that we have 3 options:

1. Special case _every_ template which operates on `TypeTuple` so that it can take functions.
2. Create a template which wraps functions so that they can be used with templates operating on `TypeTuple` without changing any of those templates (i.e. `Lambda` or whatever we want to call it).
3. Just not care about being able to use functions on `TypeTuple`s of values.

Personally, I don't think that #1 makes sense. It's a lot of code duplication which could be easily dealt with by adding a single wrapper template per #2. So I think that the question is simply whether or not it's worth adding `Lambda` to enable operating on `TypeTuple`s of values using functions as predicates with these templates. Given that it's a single template, I think that it's worth it, but I don't know. And if the problem is the name, then feel free to think up a better name.
"data-style" (or something prefixed with "data-") would most closely match what we're doing with other special attributes. Right now we have the rule that any unprefixed property corresponds to an actual HTML attribute, whereas "data-" is something that is handled specially.

(however, note that the prefix might change: https://github.com/dart-lang/dart-web-components/issues/91)
We don't. This was forcing us to have to specify something silly like "--insecure=True" rather than just "--insecure."
So it's only done once. We could move it into the method, but if people
want to be able and take and use this as a base, it makes sense to only
read and create the DOM once.

On Thu, Aug 2, 2012 at 4:12 PM, Pete Muir <
reply@reply.github.com
> wrote:

> > +import javax.xml.parsers.DocumentBuilderFactory;
> > +import javax.xml.parsers.ParserConfigurationException;
> > +
> > +import org.jboss.as.quickstart.cdi.extension.model.Creature;
> > +import org.w3c.dom.Document;
> > +import org.w3c.dom.Element;
> > +import org.xml.sax.SAXException;
> > +
> > +/**
> > + * A simple CDI Portable Extension to "inject" values from XML into the
> instances of a bean.
> > + */
> > +public class CreatureExtension implements Extension {
> > +    private static Document document;
> > +    private final Logger log =
> Logger.getLogger(CreatureExtension.class.getName());
> > +
> > +    static {
>
> Why do this as a static?
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/jboss-jdf/jboss-as-quickstart/pull/324/files#r1297969
>



-- 
Jason Porter
http://lightguard-jp.blogspot.com
http://twitter.com/lightguardjp

Software Engineer
Open Source Advocate
Author of Seam Catch - Next Generation Java Exception Handling

PGP key id: 926CCFF5
PGP key available at: keyserver.net, pgp.mit.edu
You don't need to allocate a new set here: `BookmarksInsertionManager` copies the contents.
Pokemon-exception-handling
now that I see it... considering the law of diameter it might be a good idea doing something like source.alignRevision(ci). WDYT?
here you could also check for very small/0 r.
Well ... I've already explained why I'm suggesting this and I don't want to just repeat it with other words. So, try this and see if it works:

    ...

    if ($select_override === $this->_count_string.$this->protect_identifiers('numrows') && count($this->ar_select) === 1)
    {
        $sql = 'SELECT COUNT('.$this->ar_select[0].') AS '.$this->protect_identifiers('numrows');
    }
    elseif ($select_override !== FALSE)
    {
        $sql = $select_override;
    }
    else
    {
        ...

Just looking at it should explain what I mean. :)
`(Query|Request)?Param` even afaik ;)
That looks fine.
This code will give you an array back which seems to break the subsequent code.

There is however another problem with this approach: Matching downloads by URIs won't work a lot of times: If this is the 30x channel, then the URI most likely won't be in the list of mirrors, etc.

I therefore propose another algorithm:
- If there is a full hash known already, try to match that
- Else if there is just a single download in the metalink, then just assume it is the correct one.
- Else if try to match the redirect channel OR the final channel URI
- Else abort.