labofsahil/github-event-dataset-2012 huggingface dataset:
That is exactly what the line below says in code, if you add comments, they should help explain, not just narrate what the code says anyway.
The logic is incorrect here. It should be `msg_id && (user_id || author_id)`, but this is `(msg_id && user_id) || author_id`.
I was simply thinking an integer constant would be better than returning the error string because it allows more flexibility. Instead of being required to use the provided error string, you could use your own based on what integer was returned. But yes, it could just return the word 'PHP' or 'phpBB' and have the same affect.
@p This here can now create cron tasks automatically using dependency injection with the right parameters.
Correct.

I do not know what order it pulls in from, that is decided by the finder. The paths are added to the root array it searches through there in the phpbb_style_extension_path_provider class in the find() function.
The /RACKSPCE/ shouldn't be hard coded in here since gutsy is open source and presumably someone one day besides RACKSPCE may want to use it.  We probably do this elsewhere.  If so, we should maybe just fix all of them in another pull request.
d2f7799
No, I'll remove. I meant I needed to test with some binary data. Which I've done using the composite UDFs, right?
Why not just html?
Is it cross-platform to write such find statement?
`parseAngle` should also allow numbers (that are handled as radians). That would allow us to parse the parameter independent from the type. `var radians = tools.parseAngle(param);`
Please don't bound the check to registered. You can write the same code as

```
Array.wrap(node(:nameservers)).map do |name|
    Record::Nameserver.new(:name => name)
end
```
Даже с учетом того, что для csv они не нужны
That would be okay for now.
didnt spend much time on this, its just that the margins were essential in order to separate the labels and i had changed the text color to white since black wasnt very visible on blue.

have removed the colors, i agree that they should be styled in the base css, but have kept the margins
Use Context.getConceptService().getDrug(drug_id)
I rewrote this sections to point to the other section. This will come in a following PR.
It is added at the beginning of this function.
Does it make sense to normalize case here? It's not clear to me it's always done in all other cases.
Yep, you right. I'll take care next time.
I guess I'd be happiest if we just had the util.Date option and ignored the sql.* options. My main driver is that I don't want to add a million methods to VoltTableRow. We could be "agile" and only put them in if somebody made a stink?
i is used below the for loop.
This line really should be 

    m_uniqueIndexes.erase(iter);

if you don't want to crash someday when removing unique indexes.
I missed this in the original liveindex code review and only spotted it later during a code merge.
Support yes. I'm not sure how good the test coverage is. I'll add it to my list.
ああ、これやっぱり権限でした。ちょっとそこら辺もいじってみます。
This one is strictly not related to the GCODE vs. G-code issue, but since I was touching this label anyways - I thought I might as well change it to reflect the fact, that this tab now contains much more than just start/end G-code settings.
Here's my proposal:

- instead of loading Migration in Extensions, load it in Install/Config/bootstrap.php

```diff

diff --git a/Config/bootstrap.php b/Config/bootstrap.php
index e71922a..438c5ac 100755
--- a/Config/bootstrap.php
+++ b/Config/bootstrap.php
@@ -68,7 +68,7 @@
 		file_exists(APP . 'Config' . DS . 'croogo.php')
 	);
 	if (!Configure::read('Install.installed') || !Configure::read('Install.secured')) {
-		CakePlugin::load('Install', array('routes' => true));
+		CakePlugin::load('Install', array('routes' => true, 'bootstrap' => true));
 	}
 	Configure::write('Dispatcher.filters', array(
 		'AssetDispatcher',
diff --git a/Plugin/Extensions/Config/bootstrap.php b/Plugin/Extensions/Config/bootstrap.php
index 650897e..69f3ce0 100644
--- a/Plugin/Extensions/Config/bootstrap.php
+++ b/Plugin/Extensions/Config/bootstrap.php
@@ -41,10 +41,3 @@ CroogoNav::add('extensions', array(
 		),
 	),
 ));
-
-if (!CakePlugin::loaded('Migrations')) {
-	CakePlugin::load('Migrations');
-}
-if (!CakePlugin::loaded('Settings')) {
-	CakePlugin::load('Settings');
-}
diff --git a/Plugin/Install/Config/bootstrap.php b/Plugin/Install/Config/bootstrap.php
new file mode 100644
index 0000000..4be9512
--- /dev/null
+++ b/Plugin/Install/Config/bootstrap.php
@@ -0,0 +1,3 @@
+<?php
+
+CakePlugin::load('Migrations');
```
Nitpicking: I would have reused the test_invalid_project_name test, looping on "for project_name in ('7testproject', '../testproject'):". This would allow us to test more names with less code duplication.
you don't need to use ``else`` here as the if returns.
and a space is missing after the if
I found it to be easier to read if a more descriptive atom was used for the "null" value of this field. See my first comment about this being a simplistic state machine, we transition between the nofragments, initializing, receiving and finishing states.
@jdevera I agree that it should be a good idea to access the full bundle config. I use this method just because I do not want to change too much code unrelated to this feature.
This is just me nitpicking but grammar should matter. I think you should say "This should be the bare minimum..."
The JaCoCo agent (and now the new offline runtime) becomes part of the classpath. To avoid interference with the classes under test this has to be moved into a distinct package. In particular this allows to test JaCoCo with JaCoCo.

Why random? I really want to make sure nobody start using the agent as an API. Now as we have offline instrumentation the random package name guarantees the instrumenter has the same version than the jacocoagent.jar. Strictly speaking random is not required, e.g. the commit hash would also do the job.

Do you think it is possible to generate an identifier which can be used in core and agent.rt?
import name ordering. Check out HACKING in any of the openstack projects.
Every invocation of test.js sets this variable, and our tests should always clear it so this kind of masking doesn't happen—but they don't. And I didn't want to mix those fixes with providing this hook.
Should the convertion be handled inside the default function , so one can decicde per use case if it is required?
Many original data sets form UCI or somewhere else use '?' or empty string to denote NULL values. During encoding, we need convert these values to NULL. 

This function is used to identify these fields with null values.
Two things:

1. It's support.mozilla.org now.
2. the url should totally be a link.
"from foo import *" is bad.  It leads to people getting the wrong module from the wrong place.  Only import the things you need (which should be made easier with my comment from line 368 ;).
I'm curious about the source of this one, couldn't find an upstream ticket. May be from Macports.
Yep I can revert that to "raise" if I add a separate `xcode_installed?`. I like that idea.
Hm, that can be problematic in this case.

```ruby
depends_on 'automake' => :build if MacOS.xcode_version.to_f >= 4.3
```

is used because Xcode 4.3 is the first Xcode that stopped shipping autotools, but that's also true for the CLT. If CLT-only installs return a value lower than 4.3 then they won't get the deps they need.
I'd prefer to pass an exception class rather than a string if that's possible (and straightforward).
Style comment, feel free to ignore.
I find something like the following a little more readable than what you have here:
```python
foo = blah('\n'.join([
    "line1",
    '"ine2",
    ])
```
It would also let you use `r""` and avoid double-backslashed newlines.
Hmm why we need 2 templates which are containing the same thing?
Macht funktional keinen unterschied aber  waere hier nicht logischer $lang statt $lg zu nehmen?
because unset (None) means the user can wiki, only False makes it so the user cannot wiki.
Do you want to change SAVI repo to github savi-dev?
Hi,

The eclim completion stuff used to be in the java bundle before, but since
emacs-eclim is now not java-exclusive anymore I thought it made sense to
move it.

Cheers,
-- Fredrik


On Thu, Dec 6, 2012 at 9:03 AM, Yves Senn <notifications@github.com> wrote:

> In bundles/completion/bundle.el:
>
> > +    (case cabbage-completion-framework
> > +      ('auto-complete
> > +       (require 'ac-emacs-eclim-source)
> > +       (ac-emacs-eclim-config))
> > +      ('company-mode
> > +       (require 'company-emacs-eclim)
> > +       (company-emacs-eclim-setup)))
> > +
> > +    ;; If we are using the snippets bundle, append the eclim
> > +    ;; yasnippets dir to the list of yasnippet directories
> > +    (if (and eclim-use-yasnippet (cabbage-bundle-active-p 'snippets))
> > +        (add-hook 'cabbage-initialized-hook
> > +                  (lambda ()
> > +                    (let ((dir (concat (cabbage-vendor-library-dir 'eclim) "snippets/")))
> > +                      (add-to-list 'yas/root-directory dir t)
> > +                      (yas/load-directory dir)))))))
>
> @fred-o <https://github.com/fred-o> what do you think about moving this
> into the java bundle and then to check if the completion bundle is active?
>
> (cabbage-bundle-active-p 'completion)
>
> @jone <https://github.com/jone> what do you think?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/senny/cabbage/pull/173/files#r2331865>.
>
>



-- 
==============================================
What am I up to? http://twitter.com/appelberg
Maybe use "Beitrag" or "Eintrag" instead of "post/s"
getQualifier? It's weird for a validate method to return a value. Less weird for a get method to do validation.
I don't like the implicit here, actually.  Just apply would be fine in my opinion.

Implicits functions/vals to give instances for typeclasses, make sense to me. To hide type conversion, I don't like as much.  I'd prefer we just had to do:

KryoCodecSerializer(codec)

or perhaps rename it to def asKryo(c : Codec[T,Array[Byte]]): ...

And then import it, so we could do:

register(clazz, asKryo(codec))

I think that is less magical.
Yes, Hydra now suddenly supports dates properly. This was required for caching to work correctly
Remember that we use triple-equals comparisons such as === and !== here! 
We should before too long start applying jslint to our GPII code - http://swarm.fluidproject.org/jslint/
I'd like to see this happen in JUnitCore.main.  It should be possible to create a RunNotifier that does not include the external listeners.
Again. Consider this one as feature request.
Good catch. Code should have used "matches[2]" instead. Fixed.
Oh, I see.
If Dreamweaver is implemented so, I must follow that style (because this is ADOBE brackets!)
I modified.
Please [see](https://github.com/shumpei/brackets/commit/e03bdaa017cfb5921ca8fe6a636c0ffd352dbeb5).
OK, I shifted icon down 2px. Let me know if you think it should be tweaked some more.
This RegExp is used multiple times, so it should be stored in a variable.
I can only guess why, I just tried this code with both html and css hinting. With this implementation the hints disappear whenever you press a key, that is *not* handled in shouldShowHintsOnKey (in your example the '<' will trigger+show html hints, but the 'h' will return false in SSHOK) - for css hinting this kind-of works, because I (naive thinking) added the whole alphabet a-z in SSHOK to return true, thus the hints stay visible.

To get this working one must change shouldShowHintsOnKey to include all keys (e.g. a-z) when to show hints in every provider - though I'm afraid this wouldn't solve the problem since every provider would return true and again every provider calls getQueryInfo, as far as I understand this change.
Regards, André
You dropped the `+ 1` here in your latest commit.  The command won't work without it.
(Pre-existing) typo: should be something like "Failed to normalize"
Also, we should add unit tests for the cases in #2138.
Yes, that's even better
"pseudo.class"=>"pseudo.classe" typo?
See comment on PublishPropertyServlet
I'd love to just delete this entire adapter. The whole hardcoded "failed" queue everywhere seems like an antipattern. A separate failed queue per job queue seems like the way to go ;)
list comprehension not necessary
It seems like the gray background of the sidebar isn't tied to the actual #sidebar element. It appears that the gray/white background is done using background-image in the hq-double-col class, which seems to be attached to an element that covers the entire page(minus header and footer). Removing the element seemed to be the best way to get rid of the gray bar.
Shouldn't this be a github type recipe, not git?
This is the whole idea of using mongoengine so that we can use extensibility of classes.
Links are not done like this.
You should probably do some work to escape the text, or HTML will be an issue...
find() can handle arrays just fine, just delete this block
It is. https://webmademovies.lighthouseapp.com/projects/65733/tickets/2213
Same as https://github.com/mozilla/butter/pull/1236/files#L1R205
I do have real code that needs this; that's why I introduced it.  To implement a transaction with redis, I do something like this (error handling skipped for brevity):

    ac.SyncCall("watch", "accountbalance:123")
    reply, _ := ac.SyncCall("get", "accountbalance:123")
    n := reply.Elem.Int64()
    // process account info; let's say it needs $10 added
    ac.Call("multi")
    ac.Call("set", "accountbalance:123", n + 10)
    ac.Call("exec")

I'm new to redis, but as far as I know that's the only way to ensure that the "set" call only succeeds if the key has not been modified since the "get" call.  It is useless to do all of this using queued calls (ac.Call), because the result of the "get" call would not be available until after the "exec" call returned.

My use case is a quite a bit more complicated than that, but the need is the same.
Does this bubble up to the UI?
%i->%ui
That's a no-op now and can be removed.
From my perspective this is wrong, you compare string. It means, that
```test``` convert to ```test```
but also ```test a``` change to ```test a```
only part when you have something different is
```test'a``` that change to ```'test'\''a'```
That looks wrong for me.
Ad traling slash.
```
Starting httpd: Syntax error on line 39 of /etc/httpd/conf.d/katello.conf:
RailsBaseURI must not end with a slash (/)
```
And trailing slash in Directory should have no effect. But I will try.... Indeed no effect.
Please avoid using "should".

    it "respects...
Use _groups.Clear()
@bcotton This forces use of the snappy gem. To make it optional, applications will have to `require 'snappy'` themselves.
shark.build.fileset 函数可以方便地完成
There shouldn't be a trailing slash here
I would agree with ``string`` being renamed to ``format``
Nevermind--I see now reading the code that this is supposed to work.  The question is: Why have two ways to do it here?
You should add a note that h5py is necessary and link somewhere in this docstring to http://code.google.com/p/h5py/ and/or  http://code.google.com/p/h5py/ so someone looking at this knows where to find it.
It doesn't seem like service_up/3 is used by anyone else; is there another reason it should be exported?
It does, intentionally.
@sipa I also proposed that in #1832. I'm for explicitly constructing and destructing the global objects, such as logging and db, enforced by assertions. This makes thiings more predictable and maintainable than armoring against all possible non-determinism (which is pretty much impossible, you will forget something).

Then again, it's good to have a fix in for 0.7.1. ACK.
ill remove the extracts and write out the parameters, although it means having many arguments, which i dnot like either personally
null is not a valid type, its a default value of the parameter, should just be "array" as the type in my opinion
"Utilisez CTRL pour faire des sélections rectangulaires."

This message should short enough to be added directly to the status bar. Also, the message should be different on a mac (CTRL -> META). As META isn't quite clear for people we should probably use the corresponding symbol.

Another note: the red color is usually used for error and warning messages. We mentioned using italic when we discussed the UI all together.
`Ext.apply` behaves correctly when it passed `undefined` as the second arg.
I would prefer to use a visual cue for being logged out. I will take a look and see if there's another way.
Yes. The other solution was to hack into any place where reflection was used and add checks. This one is way better :)
We should take this out and put it into the `discussion.activity` method so that it doesn't return comments that have been archived.
Let's avoid spaces in strings here `gm-aborted` and `inflight-unanchored`
If I put it in GitTfs.Vs2010/TfsHelper.Vs2010.cs, there won't work with GitTfs.Vs2010/TfsHelper.Vs11.cs (and that should be...) :(
I don't want to duplicate the code. Do you have an idea?
And I will be obliged to do an ugly cast (because I have only access to the ITfsHelper...)
Where is this line in the corresponding file?
A "dependent" package?
Isn't this equivalent to using `mode='string'` on the line above?
The test data came from rackspace-UK, with a copy for rackspace-US to see what TZ information is coming over, to verify it is still UTC.

If the format of the response is changing from an easy-to-parse one to something else, I'd consider that an unfortunate regression. Please can you supply a full copy of the response so that it can be used in tests.

Also: providing a delta-from-now would be better, as the retry-after timestamp contains two assumptions about the client: that it's clock is accurate, and its TZ setting is correct. As anyone who plays with VMs will know, time behaves oddly in this world.
Please use an explicit encoding
nrhd! なら、dev_signout と合わせて development 専用だよという意味で if で囲むとよさそうです。
The existing code checks the `-diff` attribute before doing any loading, so that is already in place. Also, we check the first 4K for binary-ness as well, but only once we have decided that the file should be loaded. This is done after the entire blob is loaded into memory, however.

My intent with this "don't diff big blobs" was to emulate the `big_file_threshold` from core git, but I guess that threshold doesn't apply for diff.

My goal is to give the user a way to stop the diff processing as early as possible if the blob is really large. It is possible / likely that the first half of the diff will have to be loaded into memory since there is a good chance that is coming from a pack file. However, I can still prevent the need to load it into memory twice.

I'm working on an update that will move the big file threshold into the public `git_diff_options` as a `git_off_t` type. I'd like to keep the default as is (i.e. a file over 512Mb will be treated automatically as a binary file). The user can set an explicit threshold, though, if that is not a good choice. Also, I will make it so that entering a negative value will turn this feature off. Does that sound like an acceptable behavior?

I'm open to having the feature default to off (i.e. a zero value for the threshold is off instead of a default 512Mb level). I don't like that as much, but I could understand why some might prefer it.

Also, I'm thinking about adding an internal API that will return the whole `git_odb_object` if `git_odb_read_header` has to load it all in. This will allow me to check the header with no repeated work even if the whole object has to be loaded in.

I'll isolate all this work in a couple of commits and if you decide you hate it, we can just reset and rip it all out.
This line doesn't compile either. The function you're calling doesn't exist. (At least, not in a form that takes an integer)
Storing the webtrends info in a variable would be nice too in case it ever changes.
Docstrings would be nice because I am so lost right now. :P
This is optional since you didn't write this, but I can't find any tests for the interpolation.
Seems like this is a todo item for upstream contribution, not for MozTrap itself, so IMO it doesn't really belong in the MozTrap codebase.
s/responsiveJoin/_responsive_join
I've replaced it now with:

```python
                when = last_success
                seconds = convert_frequency(self.config.frequency)
                interval = datetime.timedelta(seconds=seconds)

                while (when + interval) < now:
                    when += interval
                    function(when)
                    yield when
```
coming to a commit near you soon!
Actually, it ensures that the line breaks stay in.

On the review entry form, there are two ways that you can edit the review. One of them uses javascript and gets the raw HTML content of the textfield. Unfortunately, that one less than helpfully displays line breaks as "<br>". This catches those and reverts them to proper new lines if the user doesn't correct them first.
Why length+1?
comes from vim :)
you're duplicating this work. . . lines 330--339 are very similar to 312-317. Extract a method that returns the callbackType or null?
Does this trigger if I call equals() on a service interface?
why copying the file from Propel ?
consistent with https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Util/FormUtil.php#L17
please add space around `=>`
Please use space for the indentation, not tabs
why doing a special case for a serialized ``false`` value ?
this appears to be recalculated for every row that is rendered. I'm not sure about the performance, but I think this could be improved
No need to mock it. Just use the NullSeedProvider if you want.
Yeah sure, I don't mind such a flag.
But I still think it would be nice to have a fluent API to set the filename, etc. (without having to use directly makeDisposition of course).
Anyway, let's see what @fabpot wants :)
should it be `[^cgp]` ?
Initialized => Initializes
assign is the default so just omit it
`jsonDict` is confusing with `json`. Replace `jsonDict` with `payload` or remove it and use just:

    NSArray *venues = [[json objectForKey:@"payload"] valueForKey:@"venues"];
Thanks, Thomas.
I was under the impression the `type: request` was for the acceptance tests *only*.

Is that true? I'll dig myself into the docs and check...
@cjsaylor I guess an `array_merge` is better for this case. It avoid issues with arrays using numeric keys and for the cases where you want to override some key.
bad indentation.
Newline after speech marks.
mmm maybe it's just me but the syntax num_workers = property(_get_num_workers, _set_num_worker) looks cleaner. If you're ok would be cool to use this syntax.
Both ways work (thr_id and *wr) I just prefer being able to see what it is doing since it is rather critical at that point (but that is just my preference)
"thr_id >= 0 && work_restart[thr_id].restart"
one too many `'` here
I don't think we should make overriding the context menus of editors via plugins an encouraged practice. But that is a discussion for another time.

Personally, I think having buttons is more discoverable and more intuitive than context menus. Furthermore, I have seen this in several applications, but never splitting via context menu. But we should leave it to the UI people to decide what is more intuitive :) For now, let's go with the button(s), like you suggested.
应该可以这样：
```Objective-C
searchHistory = [[MSMutableArray alloc] initWithArray: array];
```
另外这个类里的`searchHistory `和`matchedHistory `其实rename成下划线开头的会比较符合code style
Space before paren
Just add comment to code that this is here just to fool test as 'endpoint' is missing but legacy subsystem also works without it.
The removal should be done in a finally block, or alternatively the setup / teardown should be done in a ServerSetupTask.
Probably good idea for master but not for 7.1
ah! yes. Thanks for taking care of it.
Clearly search and replace error.
Please expand ">" to "greater than".
Is there any reason to move this to the very top? If not please remove this change.
After commenting it with @jmcanterafonseca , he is fine about leaving as it is ;-)
we need to do something and hide the existing notification(if there is one )
the indentation seems broken too.
I've tested that if you click quickly 'back' button there is a gap of time when you could get a wrong list of threads... That's why I callback only when Im sure that the list is completely updated (Im testing weird scenarios in order to avoid having our QA yelling us next week ;) )
To follow coding guidelines,
```javascript
if {
} else {
}
```
really -1000em?
Let's either remove this line or put it into the closure?
That will make event handling much easier!
This variable is only used in the nested function, so you could move it there.
It's fine in `lib/janky/github.rb`.
I don't think so. 

In my tests it didn't work then and always returned BaseForm, e.g.

    >> BaseForm[12.3, 2]
     = BaseForm[12.3, 2]
thumbnails_bucket.dn.qbox.me
Deprecation warnings have been added.
Compare this to the other Statements, the tabbing is quite similar.  Why are you being so reticent about this?
and move it down into the "it" blocks because this is the "action" we want to test ;-) this shouldn't be in the beforeEach block
im not with you
Could you start this once it becomes necessary instead?
Again, maybe this should extend FileSource?
Makes sense. 

I suspect this might break if some kind of nontrivial copying is done anywhere in get_simple_model_features (even when the configs are None).   Of course, the question in that case is whether to change the code or the test (perhaps we WANT to have the test fail if data is copied etc).
/s/retrives/retrieves, and a period at the end.
return error/throw exception, don't end program
@pborreli why then you say it was Rackspace? assuming things eh? I caught you! :100:
nice double-space
```coffeescript
type =
  if type is 'name'
    'username'
  else if type is 'md5'
    'image'
  else
    type
```
```coffeescript
      value = encodeURIComponent value
      return
        if archiver is 'foolfuuka'
          return "#{base}/#{board}/search/#{type}/#{value}"
        else
          if type is 'image'
            "#{base}/#{board}/#{type}/#{value}"
          else
            "#{base}/#{board}/?task=search2&search_#{type}=#{value}"
```

also, are you sure the md5 search for fuuka works like that?
would it make sense to refactor this to a function, rather than a switch statement?
I reccomend removing canTakeHealth and canDie and simply make it canBeHarmed.

Remove the example javadocs below as we really don't want Vanilla concepts in the API :p
ime Google does get bored, but only after a number of successive 302's, and whilst 301 is cached by many user-agents for the duration of a session, 302 isn't cached unless it is sent with an expiry header in the future.
Would it not make more sense to set a skip attribute on the test class?
The idea is that `blamelist` is data extracted from the changes, but the who attribute, rather than the uid. So if we are extracting user information from the db, it will be duplicated with `blamelist`. The existing code did

```python
owners = [e for e in build.getInterestedUsers()
                    if e not in build.getResponsibleUsers()]
```
So rather than combining the to lists only to have split apart again, just provide them separately, and let the caller combine them if they want. (which the lookup codepath still does)
fullfi**ll**
The package $package _is_ already required?
This is very vague. There should be a second paragraph, explaining why (and when) you would want to use the hook, and what the git hook does.
this one looks innocent too
Missed a space here :)
"secure_queries"?
No, it's important to clarify that the output argument is not `OUTFILE`, but more properly `OUTDIR`, b/c the notebook *must* be exploded into multiple files for many output formats.  TeX, for example, can't manage embedded figures but instead figures must be kept in separate files.  For that reason, outfile should only, always be interepreted as a directory.

And furthermore, I think that rather than

```
convert IN OUT
```

the more useful API we want is

```
convert  --out build/ *.ipynb 
```

to convert multiple notebooks in one shot.  So the output destination should really be understood only as a directory, and be specified via a flag by default.  We *can* for the sake of convenience do what `cp` does, which is to interpret the last positional argument as an output directory, but only if it's an *already existing* directory in the filesystem.  That would let you say

```
convert *.ipynb build/
```
Sure, relative imports are the right approach here, for the reasons @dwf stated.
Is the extension of AbstractDescribableImpl<FileCallableWrapperFactory> really needed?
I think it is, because you expect each to have a `DescriptorImpl` marked with `@Extension`, right? At least that is my understanding of what `Jenkins.getInstance().getExtensionList(FileCallableWrapperFactory.class)` will expect. (`ServiceLoader` is much more straightforward.)
The button text has no contrast (blue on blue). Can we make it white? Might also have to check the color when clicked or hovered...
Would `model` make more sense as the name here?
this is only used for web apps, so I will remove the feature, it is not supported.
Might be worth doing following instead:

```js
if (require.main === module) main()
```
Even if I see the value of using `promised` on all these methods,
I would prefer introducing it only when we are going to hit such usecase.
It feels weird to wrap all methods with such magic method.
(In my PoV it would complexify debugging/comprehension/documentation for simple cases)
I agree that putting module.path in loader.js isn't reasonnable solution.
But we may have to modify loader.js in order to be able to have a custom Module class for the SDK.
Or modify it somehow in order to be 100% manifest based and do not have this uncertainty on this path retrieval.
If you feel confident enough we can postpone this to manifest modification.
The blurb with the Persona Dialog could use some bottom margin/padding around the image.  ![Image needs bottom-margin](https://img.skitch.com/20120618-gagw9ypi9hxnaea7n6u4s499rf.png)
This comment is wrong, there is only 1 failed dialog opening.
Android uses java6. Please remove diamonds and do not use any java7 features
As far as I can tell from reading the code, this doesn't actually evaluate the code, it queues up objects that later evaluate it.  If so, this should change name to reflect the actual behaviour.
Great question, let me give that a test!
`lo utilizzerà` -> `utilizzerà tale oggetto`
`visualizarla, con le variabili ed il template da questa.` -> `produrre un output tramite il template e le variabili assegnategli.`
the api value of 0x101C is deprecated so you don't need this line.
We haven't been doing destroys yet, suggest removing this for now since someone could just remove() the element.
Morten...
this should be removed as all Mink 1.4 deps are now on packagist
I would remove those spaces, as it not make code more readable.
I thought the benefit in performance is worth it. Isn't it so?
Another reason to remove it is that it should've been glColor3f(1,0,0) or glColor3d(1,0,0)
Oh, ok. Changed all .equals to = in nominal.clj.
you don't need this. Sf2 already handles it thanks to the special ``_format`` parameter. You can simply if as ``return new Response(json_encode($result))``
shouldn't parent listeners be inherited too (by merging the arrays together) ?
That's available at https://github.com/12dCode/doctrine2/commit/465778830c4068dfc3df0e075959a61829c23526
you fix? :)
package name without caps are usually prefered
then I'd settle for something like:

```javascript
for(var k in obj2) {
   if(obj2.hasOwnProperty(k){
      obj1[k] = obj2[k];
   }
}
return obj1;
```
str.format not in 2.4
For reference, this is what the traceback looks like (for a hobby site of mine):

    Traceback (most recent call last):
      File "/Users/kumar/dev/recordbox/vendor/lib/python/nose/suite.py", line 227, in run
        self.tearDown()
      File "/Users/kumar/dev/recordbox/vendor/lib/python/nose/suite.py", line 350, in tearDown
        self.teardownContext(ancestor)
      File "/Users/kumar/dev/recordbox/vendor/lib/python/nose/suite.py", line 366, in teardownContext
        try_run(context, names)
      File "/Users/kumar/dev/recordbox/vendor/lib/python/nose/util.py", line 478, in try_run
        return func()
      File "/Users/kumar/dev/recordbox/vendor-local/test-utils-001f212/test_utils/__init__.py", line 159, in tearDownClass
        cls._fixture_teardown()
      File "/Users/kumar/dev/recordbox/vendor-local/test-utils-001f212/test_utils/__init__.py", line 214, in _fixture_teardown
        transaction.commit(using=db)
      File "/Users/kumar/dev/recordbox/vendor-local/django/db/transaction.py", line 138, in commit
        connection.commit()
      File "/Users/kumar/dev/recordbox/vendor-local/django/db/backends/__init__.py", line 223, in commit
        self._commit()
      File "/Users/kumar/dev/recordbox/vendor-local/django/db/backends/postgresql_psycopg2/base.py", line 235, in _commit
        return self.connection.commit()
    IntegrityError: update or delete on table "keyword" violates foreign key constraint "keyword_id_refs_id_401b1e47" on table "x_ebayauction_keyword"
    DETAIL:  Key (id)=(92960) is still referenced from table "x_ebayauction_keyword".
Prettier to indent.
we could use 
<code>return self.is_element_present(*self._mission_sidebar_link) and/
self.is_element_present(*self._forums_sidebar_link) and/
self.is_element_present(*self._governance_sidebar_link) and/
self.is_element_present(*self._history_sidebar_link)</code>

if one of the checks if False the result will be False
the default way to render a foreign key field in a model form is the choices widget. in the __init__ method we override the choices.
Should call this HTML5 and not object.
typo: s/randon/random
Looks like you have a couple of return True in several functions that is not needed.. Maybe simplify and remove them?
Is it necessary to have extra mixin just for selected tab color? I think that for this non-repetitive element, we should use global mixin for linear_gradient:

@include linear_gradient($tab_selected_color, $white_color, $tab_selected_color);

it is part of the patch, which I was sendin to you couple days ago.
I don't understand what this method is doing.  Is it building XML?
The concrete5 structure is extremely rigid as it sits, we weren't criticizing your methods, we were just pointing out a typo =p
Why is `namespaced_lock` defined with the same behaviour `lock` used to have, but only use `lock` in `namespaced_lock`
If you can remove this debug message I can merge this :) Just a minor tweak..
we usually try to avoid to define function in the global scope
if the filename contains dot, so just use dir as baseURL
just to avoid casting warnings in scintilla_send_message as it doesn't expect GtkWidget*
I would probably prefer to have this still as `@repo.users.count` as I don't want the repo.rb to turn into a junk drawer of helper methods and there's not a ton of work going on in that method (it actually takes longer to type). If we were using it multiple times on the page we could memoizie it with `@sub_count ||= users.cont` but it's wasteful if we're just using it once. 

This looks good for now, just wanted to throw in my two cents on this style :)
What about the verifyEvents() below?
Just realized our server output isn't showing debug lines.
I think this could just be `print_func = console.log`
I'm assuming this is an array because you want the order to be preserved? Otherwise, {'id1':'Value', ...} would be simpler, but the order probably requires it.
Additionally, we should limit the docs to only OAE modules, and only api.js, model.js and rest.js .
This should first check if the group name already exists.
This test isn't as valuable anymore with all of the tenants created in the tests above.
This is a lot of extra code just to deal with streams. Will there be many cases where we need to support streams? If not, is there a better way to do this (e.g. a RestStream function, etc.)?
Yeah, it looks like jquery.jeditable is already in node_modules/oae-config/public/lib/js. This should be removed.
I copied this test file from _test/tests/inc/auth_aclcheck.test.php as a working example, so I didn't notice that the parent::setUp() is recommended.

By the way, almost all test files in _test/tests directory have no parent::setUp(). Do I really need to fix this?
Shouldn't this have a $priority value, like RouteFilters?  It's inconsistent otherwise.
no constant for 3600?
This is good. You might want to rename "breaker" to "delimiter".
should add

  this.options.onProgressChanged = this.options.onProgressChanged || function(){};
Also updated to use appender
Here's your problem. You're checking the matchup for a to the matchup for b. instead you need to check matchup for a to b.
Да, в этом случае нужно просто перемешивание массива в случайном порядке, даже если элемент останется на своём месте - ничего страшного. Перемешивается массив индексов и мутация будет проходить как надо.
Везде счёт идёт от 0. А количество - это количество элементов. При количестве 2356 элементов в массиве индекс последнего должен быть 2355
Yeah, I'd love everything else, but my git-fu is weak. Is there a way I can do a targeted pull? Or force manual merge even without conflicts?
It doesn't seem to make any sense to create a `RACCommand` without a can execute subscribable or an execution block, but it does make sense for `RACAsyncCommand`.
i.e. the result should be saved in @data
This appears to already be [included in the twitter-boostrap-rails gem](https://github.com/seyhunak/twitter-bootstrap-rails/blob/master/vendor/assets/javascripts/twitter/bootstrap/bootstrap-typeahead.js). Not sure how/if we can hook that up to Buster though... thoughts?
```javascript
console.log('hello world');
```
Sorry, I see 8 lines of code, what does dd mean?
We should mention the commands, e.g.

git checkout master
git pull
git checkout -b my_feature
> I find it more natural do to:
>
> ica = ICA(n_components=25, noise_cov=None)
> ica.fit_raw(raw, picks, start, stop)

> so you can next do:

> ica.fit_epochs(epochs, picks)

ok, is fine with me. I just thought in terms of attributes having a thicker init might be a good idea.
the two most important ones are the pciks and sources_ids, let's what to do with this.

> and it will work the same relying on a hidden private function that does the ICA a 2d array of data.

good you name it, was on my list (important but not urgent)
IMO, it is not only form html form.
You can process an xml ou json form ?
it's a TODO note anyway... ;)
missing S
并且
但
去掉“地址”
Agree. It would go in backordering. Deleting old line_item makes sense. 

Sent from iPhone

On 09-Mar-2012, at 3:04 AM, Ryan Bigg<reply@reply.github.com> wrote:

>> @@ -429,6 +429,13 @@ def insufficient_stock_lines
>>       line_items.select &:insufficient_stock?
>>     end
>> 
>> +    # Copies the +line_items+ from passed order
> 
> If you're merging two orders together, wouldn't you need to nullify one of the order's line items? For example, if a variant only has 5 stock items available, merging it like this would cause it to go into backordering.
> 
> I would recommend clearing the old order's line items as you iterate through them, i.e. in a call right before `add_variant`.
> 
> --- 
> Reply to this email directly or view it on GitHub:
> https://github.com/spree/spree/pull/1231/files#r536063
Или просто `const auto& e = event.isEmpty () ? GetCurrentEvent () : event;`
Тело лямбды на отдельной строчке — +1 таб.
В таких случаях можно одним @brief'ом ограничиться.
Hay algo raro en el pdf en este punto, especialmente con los espacios del título y los espacios de los últimos items de la lista.
Hi got it, for the button on the Demo i used the same name that the button had assigned, and add the click event to it.

For the MessageBox, I used general names because the message box change the buttons name depending on the type of messagebox you decide to open, right now there are three types (OK, Yes/NO, OK/Cancel), and the control change the names of the controls depending on it.

do this make sense to you?
Seem to be missing a closing parentheses here.
constructor, it is not constuctor
I don't think you need this method, especially as it only does half of the creation
The argument could be typed as `GTRemote *` above, which would remove the need for this cast. (And the `isKindOfClass:` check will make sure that it's accurate).
These should be `STAssertEqualObjects`.
I could fix that, but IMO it is better to open a ticket (easyfix tagged?), and have someone do a proper regression tests on it.
This and the `TypeError` below stretch to longer than 78 characters. In lieu of porting the codebase to be PEP8 compliant, could you wrap these and any other lines that are too long?
1) Please specify which ones here.  It looks like all the major ones except cairo and macosx actually do support it, since all the major interactive backends are based on agg.
2) Please also note here and in the contourf_demo that the standard colorbar does not support the hatching.
Ah much better.
Can you wait for the new frame to appear and then switch to it?

Sleeps like this don't go well with the reviewers here :)
no it's using the local variable created in the method.
This is not our usual style of course, it's a legacy style from RC moztrap.
да и может быть все-е вместо $this->value[0] сделать isset($this->value[0]) ?
This is correct. I would assume

```php
if (!($dept && ($tpl = $dept->getTemplate())))
```

was originally intended
Given that you set url_postfix on line 20, @account will conflict
memcpy would be nicer here
use SG_CALLOC (then you don't need the memset)
Agreeed, we use SQL explanations elsewhere so keeps it consistent.
I think I'll change it to .cms-content-actions, .cms-preview-controls.  That covers both places these controls may appear.
It didn't work when I first tried that.
This needs to be moved into `example`.   See https://github.com/texastribune/tt_app_templates/tree/master/generic for an example structure app template structure or https://github.com/tswicegood/atx_restaurant_ratings for one that has been generated.
Should this commented-out CSS be removed?
If so docblock on available should indicate that it accepts user input.

If your only issue is code duplication I would suggest factoring that regexp out into its own function and calling it here.
Again, the `params` not having a default value (say a hash) will break backwards compatibility.
How about making the return type a bit more specific; maybe use ``array<Requirement>``?
My point is that this is not needed. Why are you adding this? This doesn't fix the bug.
IMO useragent sniffing is _THE_ way to do cross browser compatibility. Otherwise you're being incredibly optimistic that one could cover all bases by trying to find "hacks" & tricks that somehow conform to all browsers. This is only more true with bleeding edge tech that's not completely well specced by W3C.

Modernizr has it's heart in the right place, and it probably does a great job covering most cross browser problems, but in the end you'll never avoid firing up VMWare and putting in conditionals for each browser quirk.
http://www.kernel.org/pub/software/scm/git/docs/git-rm.html
Surely. It is useless
There are three spaces instead of two after $user, which the code sniffer is throwing back as an error.  The same for the other @param tag lines.
I like this, so long as it doesn't interfere with robots that are reading the page, e.g. accessibility agents or whatnot.
Also, this causes the tests to run "slightly" slower (or maybe just because I'm on slow internet). I saw an `abort` command somewhere in the code, but I'm not sure if that would be useful here.
Spaces around `()`. You can go into newline after {` and before `}`.
Spaces. Please, go over all code and check such things yourself.
So you could just append to object stored in a variable here, at the end.
Aha, for errors it is simply useful to define a global jQuery Ajax error handler. So that you do not have to repeat this again and again.
No, `default:LOGIN_REDIRECT_URL` is not good in generic form template. You set this with `with` before including the form in templates you want it.
Decide how you want variables to be named. Here you have `_xml` suffix, but it is not really a XML object. Maybe something else would be better.
How do you define default value for a MapField? Any way I try to pass in a dictionary, it just returns a `ValidationError` of

    Only dictionaries may be used in a DictField: ['panels']
explains, Python
Will do. No need to execute another conditional if one already passed.
ok. just fix it
const int uploadParts = 5;
this is a css hack is this necessary?
Type hinting please :)
for the ``else`` and ``elseif``, you should also fix previous cloding curly bracket (which should be on the same line)
I think:

```ruby
if value.any? {|_,v| v.kind_of?(Proc) || v.kind_of?(Symbol) }
  raise ArgumentError, "all custom_data attributes should be either a Proc or a symbol"
end
```

is a little clearer.

Also - I think custom_data would be a better name, in which case a slight alternative to above would be:

```
if custom_data.values.any?{|value| value.kind_of?(Proc) || value.kind_of?(Symbol)}
```
Yes `tryGetValue` does the assumption you made. Regard the naming, that's best thing I was able to get if you have another suggestion I'd like to hear it.
Space around "||".
Looks like this could be done by filtering `this._data` with a function returned by `helpers.eq`?
NGX_ERROR ?
They refer to it as LCC for Library of Congress Code, let's do the same.
can whiten and _compute_res be shared, inherited from SysModel by default?
Can you explain this change a little more? I don't follow the changes well.
Now that we're re-displaying the ad tag component on every page load, we need to handle the not-reachable case on every page load and remove the class if `isReachable` is false.
no that is the device interface class or some such (0003 = HID)
Ok.  I think this might be pretty edge case.  Not sure if it will clutter it too much.

Would be:
[ log output .....
<transaction ID> <transactionID> ....
messageID

Might make one line a little longer, not sure if I would consider that clutter and would add useful info.  Easy change to revert though so i can go either way.
Ок. С этим согласен. 
Там код очень похожий, поэтому лучше воспринимается когда можно строчку одним взглядом охватить.
is position.py (and its contents) deleted on this this branch?
I couldn't find `galsim.config.load()`?
I changed all of these (in both Bounds and Image) to xmin, xmax, ymin, ymax.
Hmm should this be here?
I know, sorry ;) To be honest, I think it's harmless in most cases, and actually useful for anyone trying to run our examples on OSX. I will post on _brooklyn-dev_ to canvas some opinion.
It was commented out for debugging, will fix
Include a TODO in the comment at least, for places like this.
> then changing to `* 0.1` is far more intuitive than `/ 5`

I assume you meant `* 0.2`. I think this one is subjective, too, but it appears as a simpler operation to me when I think of dividing by an integer instead of multiplying by some real number between 0 and 1. So, when it can be expressed that way, especially when it will be a multiple of 10, I will write it that way. But as I said, that's just me.

The leading zero is a best practice, though, as you don't want to miss the `.`, mistaking `.5` for `5`, and you don't want it to look like the start of a member access. There's probably other good reasons, but it's been a long time since I thought about *why* I always prefix the zero.
*warning:* No PHP code was found in this file and short open tags are not allowed by this install of PHP. This file may be using short open tags but PHP does not allow them.
It blows up if you remove the only_if portion with the rpm_package resource? If so, that seems like a bug.

Re: Yum
I thought yum just interfaces with rpm, so I'm surprised that matters. Have you run this in debug mode to see what exactly it's doing?
That doesn't look right
It's a bit hard to read. What about adding spaces where it should be indented in the code? (i wonder if i'm clear here)