labofsahil/github-event-dataset-2012 huggingface dataset:
OK, that makes sense, but it was this was not clear until you explained it. Please add a comment something like:

  // default behavior for url hints is do not close on select
Does this need to be set here???
or is it fine for the tree to take over this when it loads
You should have avoided automatic refactoring...
wait... what ?
``` ruby
assert klass = Rugged.const_get err
assert_kind_of Exception, klass.new
```
I would just add to the list of packages. For one other project I have simply like:

```
Mac OS X
^^^^^^^^

Prerequisites can be installed with MacPorts_ or Homebrew_. For MacPorts::

    sudo port install mongodb rabbitmq-server memcached py27-virtualenv py27-pip

and to start them at startup::

    sudo port load mongodb
    sudo port load rabbitmq-server
    sudo port load memcached

For Homebrew::

    brew install mongodb rabbitmq memcached

and follow instructions to start them at startup. You also need Python_, pip_, and virtualenv_::

    brew install python --universal --framework
    brew install pip
    pip install virtualenv

.. _MacPorts: http://www.macports.org/
.. _Homebrew: http://mxcl.github.com/homebrew/
```

So very similar to what it already is, just adding a new package for mongodb.

I have not seen that you have to create a directory for data? And are you sure that you want to run it always manually? Anyway, this could be only for Homebrew? Then write that it is only for Homebrew necessary to do this.
How else would you do it? There are already like 8. 

If we want to have it run through an array of file names or something instead, that would probably be better, although not this ticket IMO.
I'm pretty sure I agree, but I was following the setter inside currentTime above it. I have seen returns in setter before, and I *think* it is to allow code like this:

var item = obj.item = 4;

With obj.item being a setter, but also returning the value? I will test this and get back to you.
I actually use the vertically longer form whenever I code. You really don't like it? I find it much easier to digest.
Do we need these? Probably not
Why did you remove the other options?
You don't need this return
error okay?  This seems odd.
Unfortunately, someone also used `submit_row` elsewhere, requiring a second hook for this field. If said person wanted to discuss something a little more meaningful than `submit_row` for rows not involving *submission*, I might be persuaded to entertain the idea of not having a seemingly redundant `submit_row.submit` class floating around.
woah - that's a pretty nasty selector, are we able to use a more standard solution using a class?
If the package you have asked to install is just a package name, don't attempt a downgrade
I did this thinking of extendibility. I think we should keep it like -

```js
['git'].forEach(function(dep) {
    if(!sh.which(dep)) {
        console.log(('Sorry, this script requires ' + dep).red);
        sh.exit(1);
    }
});
```

What do you say?
Shouldn't need this import
As with ``data.get('tags', [])``, so too here.
Can you run these only if "support for Foreman is installed" (you mention this phrase later on). See rhsm test cli suite how we do this for RHSM.
There should be only `if defined?(Bundler)` otherwise it won't load any gem in headpin mode :)
Should we instead use the Compass mixin? http://compass-style.org/reference/compass/css3/opacity/
Can this just be:

    visit "/lenders"

?
This should be moved into the for loop, IMO:
```C++
for(unsigned int idx = 0; idx < input.size(); removeChar ? input.remove(idx, 1) : ++idx)
```
(also changed idx type to unsigned since it is compared with .size())
I didn't know that's the initial value.  In that case things look good :D
I'll change them to use the longer form.
Sample code showing how to add an AddKMLFile plugin to a `gxp.Viewer` is available in the `GoogleEarthView plugin <GoogleEarthView.html>`_.
额..这个好像是KDevelop自己生成的...
The tabs should be editor independent. Always use 4 spaces instead of tabs. Otherwise it's possible that it causes really nasty errors.
Why class and not styleClass ?
Ok! Will do it...
Do we want the profile to be configurable?
Let's use a CSS class here ('js-error'?), instead of direct attrs.
Pedantic, but I'd rather see this as `import glob`, and then `glob.glob` below.  First more with our usual style of imports.
can you remove this file from the commit?

there are tips here: http://groups.google.com/group/jclouds-dev/browse_thread/thread/899618e53cab800d/1e81e054d811e8ef
and here: http://stackoverflow.com/questions/307828/git-remove-file-accidentally-added-to-the-repository
I'd make it opaque and have something like `git_branch_tracks()` that returns which branch it's tracking (either with its remote-tracking name or some other information). I don't think there's anything wrong with a `_lookup()` that accepts `origin/master`, as long as it knows that it's remote-tracking and shouldn't be modified.
The current and advance callbacks share the same type signature so the empty iterator just reuses the same function for both, hence it doesn't follow the naming conventions that the `ITERATOR_SET_CB` macro requires. I could probably rename `empty_iterator__no_item` to `empty_iterator__current` and the `#define empty_iterator__advance empty_iterator__current` and use the macro, I suppose, although I don't feel that the resulting obfuscation is worth it for about 4 fewer (net) lines of code.
Actually, this was intentional, but I thought too small to be worth mentioning. Since with Vicent's changes HOME is not used on the Windows platform, when I rewrote these tests, I introduced a warning about an unused variable. This clears up the warning.
Looks like this string should be processed before passing to mail_error?

Sorry. First put this comment on the commit in your fork by mistake.
Couldn't find any. Shouldn't be too hard to throw in though. I couldn't find where any part of `l10n_utils.gettext` was tested.
I think you accidentally a p.
This one can also be just `alt=""`
Do we intend to support creating/editing users via the API? If not, should this resource be limited to GET?
Now I sort of see why you let the implementation (e.g. socorro/external/...) do the swallowing. Hmm... my "criticism" might have to wait :)
I don't understand why the rpush is necessary.
Overloading the meaning of a value (in this case, `email.host` can mean either "use sendmail" or "use this hostname") is generally a bad idea. In the future, use another field which says "use this method", something like `email.method = sendmail` or `email.method = smtp`.
I would always return `0` here.
should test ```[VenueInfoViewController onScreenVenueVC]``` for nil
Got an extra indentation in this block
Yeah maybe overkill. I'd go for a more detailed name for the method then instead.
Could you please cleanup the conflict (// comments) ?
Change this to "Searched the whole gallery."
you are right about it's not really last use, but it does not reap the connection because as long inUse return true as long is not returned to the pool.  

But it do raise another problem you mention, if a user forget's to close the connection and it still valid it will leak connections.

The question how do I deal with this? introduce some kind of transaction timeout, so after a specified time it will reap the connection even it still might be in use?
So the TODO should be removed?
Also are you sure this is the only way to get an IllegalArgumentException ? I would only wrap the getPathNameRelativeTo in the try/catch block, so you don't accidentally catch an exception from parseTLD
We'll have to check with the i18n/logging team on how to deal with changes/fixes like these where a typo is being fixed but ultimately it's going to result in a change to the property name in the i18n properties file but would still retain the same message id. I think it should be OK and this perhaps will be treated as a new property altogether.
I'll fix that prior to release.
Not registering the activities handlers will be easier; but in Contacts app, the window frame need to handle "new" activity and inline frame need to handle "pick" activity, so they should both register for activity and trying not to handle the wrong ones. So I put the `return` right here.
Sorry for nitpicking: oldMoveBy -> baseMoveBy
Shouldn't be this measures in rems?
The strange thing was that it didn't happen always, and I was not able to repeat it each time. But I think that trying to run app we want to close is useless, so removing the listener isn't bad in here, is it?
nit: space after colon
```css
  margin: 0;
  padding: 0;
```
Since here is not warning or error status, we can leave clear comment instead of printing the log.
Actually, you shouldn't call `EditAlarm.getCurrent` here. You can just get the object directly from DB or from the array you saved when you `fill` the UI.
good point.  I'll change it as you suggest.
I don't understand what your mapping is doing here.  IndexedDB will already accept a single string or a list of strings, and the transform through this.store only seems like it would make sense if you are dong some type of horrible toString() thing?
I love you too... :-)
nit: extra white-spaces
I don't know why cards view needs this code but maybe there's something I couldn't figure out. ping @timdream again.
There is no need for this allocation here. We should avoid unnecessary allocations in performance critical code.
hmm, now my spec doesn't pass, because it was checking for `have_` cases. I guess it's ok to return true, it will fail to find the method in the class anyway... I will update the spec too
Exists call doesn't need to be in the try, it's harmless. I could catch IOException to be more specific.
另外，可以考虑为某些特定的功能加独立的 api。比如：

rs.crop(key, url, x, y, cx, cy)

这个函数内部实现通过调用 rs.saveAs 实现，是 imageMogr 的特例。
How do you think about naming the helper method: `truncate_with_ellipsis` ?
I'm getting concerned that there is a lot happening here, and it's becoming harder to reason with. What do you think about breaking each type of source down into its own class? That way each class has a clear purpose. 

`Conformist::Column` becomes responsible for setting up these classes.

``` ruby
def values_in enumerable
  source = "#{columns.class}Source"
  values = source.new(source_columns, enumerable).values
  # ... preprocess values
end
```

Each source gets access to the `source_columns` and the `enumerable`. Because they're distinct classes, we're free to add as many helper methods as we need. Range and Integer is quite small, but all that leg work in String could be split out into isolated methods.


``` ruby
class RangeSource
  def initialize sources, enumerable
    @sources    = sources
    @enumerable = enumerable
  end

  def values
    Array(enumerable).values_at[sources]
  end
end

class IntegerSource
  def initialize sources, enumerable
    @sources    = sources
    @enumerable = enumerable
  end

  def values
    Array(enumerable).values_at sources
  end
end

class StringSource
  def initialize enumerable
    @enumerable = enumerable
  end

  def values
    # ... do named column stuff
  end
end
```

It might not end up looking exactly like this, but I think it could help here.
The reply.* interface always require a .send() to finalize. This allows us to add other operators to the rendered view such as ttl().
Presumably we should be able test something a bit more specific here, even if we don't go as far as geometry.   Is there some simple way of independently figuring out the specific value that len(within) should take on for each of these cases?   E.g. could we run it and then verify the values by hand?  That might be a bit labor intensive ... but it would only have to be done once ...
I don't get how indexes can have keys and values.
This new arg should appear in the tests for this module, as well.
That was the plan. :)
I removed it not because it caused failture
Sorry just one last nitpick that I missed before, the `_?` you added should just be a `_` added inside the `[.-]` => `[._-]`.
I'm sure you know, but just to confirm: the response in case no element is found within the given Implicit timeout is an empty array?
The spec doesn't say, and I assume it's implicit (plus, you wrote a much more complex driver - you know the answer!) :)
It looks strange to have addEntryPoint() and addWindowNode() methods in a RuleBuilder interface... it is probably something we should fix in the actual code, but since you are creating the interface, I would suggest to avoid adding these methods here.
Looks like yout've replaced spaces with tabs? why?
okay, currently I don't see any reason why use one or another way, because they both are exactly same...
your "low-level"
```ruby
 @env['PATH_INFO'] = @request.path
```

and Rack's "[high-level](http://rack.rubyforge.org/doc/classes/Rack/Request.src/M000263.html)"
```ruby
`def path_info=(s);   @env["PATH_INFO"] = s.to_s               end
```

I would prefer `path_info=`, but that's just me and doesn't really matter...
We should remove all references to this if there's now $g
Joomla.Libraries
please use: `"main": "./cfx.js"`. Also could you please add `name` and `description` ?
@ochameau  I don't have strong feeling about it, I just thought was better working on that if and when the conflict comes. However I saw you added a deprecated's function to your pull, it means this file needs to be change even if remove those two modification, because the deprecation of `getChromeURIContent`.

If you could deprecate `getChromeURIContent` in the pull/536 (and maybe add the small nit to fix the `NetUtil` at line 13) that would be awesome, and I can just rollback the entire `data.js` file. As you said, it's true that goes beyond of the scope of this pull request.
Can't we pass `loader` and assert on it?
Not sure if it's better to throw or return transparent thumbnail.
@lloyd - I like the updating of the comment #'s - there are some others that look like they could use numbers or need re-numbered.  For instance, the numbers jump straight from 10 to 13, and there are some app.use calls in between, and statsd does not have a number.
Have been updated before.
See this link :
https://github.com/nvcVincent/CoinCod/commit/0029151928d751b4c6bf30560a30d2f1278a1574
Should be /proc/self/mounts as the example fails since merging 10aa3aa.
Yes, both include? and kind_of? return true or false.
Your logic sounds right; I can't remember the details of how the exception handling works during acceptance tests but I am pretty sure that there was a reason that I had to do it that way at some point in time.

w/rt the comment, the goal was to keep a visible reminder there to try to help make sure that that actually gets addressed at some point; without it there's nothing to indicate that the code isn't entirely in a desirable state.

that said, I know that you have a different take on that kind of stuff than I do, so handle it however you like... I think at this point we're mostly just discussing the stylistic aspects of this change, and  you should be the one to make the call on those :)  seems ok to me from a functional standpoint.
What do you mean by "default check"?
When it makes sense to break a guideline, you should break it.
If need be, change the global color vars we have to be those of the alerts and what not. Those were specifically designed to work pretty well. I don't recall the state of the color vars honestly.
Ok I'm very busy, but will take some time to do that tomorrow :)
Thanks for the advices!



Il giorno 04/giu/2012, alle ore 01:07, Richard Pickett
<reply@reply.github.com>
ha scritto:

>> @@ -145,6 +148,9 @@
>>           .css(tp)
>>           .addClass(placement)
>>           .addClass('in')
>> +
>> +          triggerEvent = $.Event('shown')
>> +          this.$element.trigger(triggerEvent)
>
> The check for `isDefaultPrevented()` allows the user to call `e.preventDefault()` in their event handler. For the `show` and `hide` events, if the user calls `e.preventDefault()`, it means they want to stop the `show` or `hide` function before it happens. However, for the `shown` and `hidden` events, the action is already done and there's nothing to stop.
>
> Yes, I could do the patch by myself but it was your idea and you've done well so far so you deserve the credit for it. I'm just trying to help and I'm learning most of it at the same time as you.
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/twitter/bootstrap/pull/3691/files#r920252
I think this could be simplified a little:

```clojure
(fn [x] [(-> x meta :protocol meta :name) (-> x meta :name)])
```
Then please edit the PR title and add `[WIP]` at the begin of the title
This should be "Value" -- "self" is unrecognized by most tooling (phpdoc, IDEs, etc)
actually, it should be $this->_filter->filter.
Please use "Convention over configuration". There is convention for naming join table and you don't have to supply its name here.
Yes. We agreed about that for the admin GUI.
nit: for consistency, char *rv; ? (* rv vs *rv)
That wouldn't work because nginx runs as root anyway probably?
Should `reset()` really take a parameter? I don't think any other `reset()` method in the framework does.
Well, it's used right here...o.O
Are you suggesting the following?
```ruby
before_filter :store_location, :only => [:require_user]
```
If so I don't see how that is better.
The doc is wrong.
Using "\" in function signature prevents any static analysis with HipHop, hence why we forbid it in our CS.
I forget about that: should we remove this "static" subdirectory or keep it for backward compatibility?
two empty lines here.
Actually, that function is not used anymore. It's replaced by `process_software_build_specs`, which is more general.

Fixed.
use stl
Oh, this is not used anywhere else (from what I can see).
why does indentation change here?
Does this change the minimum version of Node which we would support?

Would be good to check if `fs.existsSync` exists.
Shouldn't legends relate to fieldsets? Otherwise, just use a header.
No need to `lower()` slugify will do that.
Slugs are only alphanumerics and dashes and underscores.
Default length is 75 if you are going to specify it.
How much of this do you actually need to define?
this slipped in from some debugging code I had. I will remove it.
Nuke this test.
Damnit, forgot a parenthesis.
Do we want an index on system_name as well?
please remove the null check. should be consistent regardless of prefix
Sorry to have missed this earlier. This should be `Logger.getLogger(getClass());`
I'm actually not sure I understand this. Is it because `Factory` is a common name and other people could have that name in their apps?
Why sendMessage()?

This cloud be queue.clear()...
Michael Sevestre <notifications@github.com> wrote:In app/views/monologue/posts/_revision_header.html.erb:> @@ -1,9 +1,14 @@
>  <header class="revision-header">
>    <h1><%= link_to revision.title, revision.full_url %></h1>
> -  <div class="posted">
> -    <time datetime="<%= revision.published_at %>">
> -      <%= I18n.localize(revision.published_at.to_date, :format => :long) %>
> -    </time>
> -    &nbsp;&nbsp;|&nbsp;&nbsp;<%= revision.user.name %>&nbsp;&nbsp;|&nbsp;&nbsp;<a href="<%= revision.full_url + "#disqus_thread" %>"></a>
> -  </div>
> -</header>
> +    <span data-monologue="time">
I removed the div class="posted" because it's not needed. Let me know if there is a need for that and I can put it back in
              —
              Reply to this email directly or view it on GitHub.
Please add a very short description next to the link.
So we still need urllib because of unquote()? Is there no replacement in urllib2?
Ok, I made this change. In the process, I also moved the `file_ext` argument from the `Scraper` constructor to the `DailyScraper` constructor, since it's `DailyScraper`-specific.
Did you get the if and else mixed up?
Why reset the reference to the setpoint?
Can you please add the default i2c port to the description? And a comment that this disables the normal electrical subsystem...
User content objects doesn't quite make sense. Isn't this an array of restContexts ?
It might be better to follow the id format here (rev:tenantAlias:shortId), so we can run isResourceId on revisions.
Can you put a comment for the different arguments that take a magic number/boolean value, e.g., 

```document.replaceWith(newContent, /*caretOffset*/ -1, /*overwrite*/ false)```
I don't understand the reason to not highlight the full unicode character escape sequence. The grammar is regular (`"\u" + 4 hex digit`), and if it is illegal at this point, the compiler will return an error (or doesn't, see [SI-6810](https://issues.scala-lang.org/browse/SI-6810)).
Could the above formatting chage be reverted?
If you wrap ``referenceType.classLoader()`` in an ``Option``, you can make the code a bit more readable IMO.

```scala
val classLoader = Option(referenceType.classLoader()) getOrElse {
  val classLoaderClass = virtualMachine.classesByName("java.lang.ClassLoader").get(0) // if there is no ClassLoader class, we are not debugging a JVM
  staticInvokeMethod(classLoaderClass.asInstanceOf[ClassType], "getSystemClassLoader", "()Ljava/lang/ClassLoader;", thread).right.get.asInstanceOf[ObjectReference]
}
```
s/getProcess(application)/CurrentProcess
On Fri, Sep 21, 2012 at 7:39 PM, Roshan Sumbaly <notifications@github.com>wrote:

> In src/java/voldemort/store/readonly/ReadOnlyStorageEngine.java:
>
> > +            fileModificationLock.readLock().lock();
> > +            List<KeyValueLocation> keysAndValueLocations = Lists.newArrayList();
> > +            for(ByteArray key: keys) {
> > +                int chunk = fileSet.getChunkForKey(key.get());
> > +                int valueLocation = searchStrategy.indexOf(fileSet.indexFileFor(chunk),
> > +                                                           fileSet.keyToStorageFormat(key.get()),
> > +                                                           fileSet.getIndexFileSize(chunk));
> > +                if(valueLocation >= 0) {
> > +                    if(exact)
> > +                        keysAndValueLocations.add(new KeyValueLocation(chunk, key, valueLocation));
> > +                    else
> > +                        results.put(key, true);
> > +                }
> > +            }
> > +
> > +            if(exact) {
>
> So if 'exact' is specified, it will no longer be an index-file-only read,
> correct? - Yes
> In this case, don't we still need to control the rate at which hasKeys
> shall be used? - define "control". Do you mean throttle? If yes, then
> "exact" being false is precisely what the user would want to use for quick
> lookups. Unfortunately the current format of read-only files does not
> permit us to do exact matches without looking at the data file.
>
If hasKeys access data files, it will introduce the same amount of load to
servers as getAll. Won't it defeat the purpose of this function? There will
be saving on network and serialization, but those costs are insignificant
compare to the server cost.

All I am saying is that hasKeys for RO is not as cheap as we thought. Use
it with care!

Thanks,

Lei


>  —
> Reply to this email directly or view it on GitHub<https://github.com/voldemort/voldemort/pull/95/files#r1667218>.
>
>



-- 
Lei Gao
Unless there's a significant reason for adding newlines, let's leave it as it is - the appending to get the window.CAMPAIGNS in the top itself is making me twitch a little bit. Treat this as a data blob instead of a file of source code? :)
Not sure if I'm seeing the intended behaviour, but when I try and display an image in markdown this callback runs continuously and no image is displayed.
No slash at the start please - that means an absolute path, which is not the case.
According to PHP manual: http://php.net/manual/en/function.strtok.php
```
<?php
$string = "This is\tan example\nstring";
/* Use tab and newline as tokenizing characters as well  */
$tok = strtok($string, " \n\t");

while ($tok !== false) {
    echo "Word=$tok<br />";
    $tok = strtok(" \n\t");
}
?>
```
Is that space really needed?
Где добавили?
Тоже не понимаю.
Да, спасибо. Упрощу.
Is there a reason why you're using `moveItemAtPath:toPath` here instead of `NSWorkspaceRecycleOperation`? I'm not sure which is best, but it seems like `NSWorkspaceRecycleOperation` is the preferred method based on a quick skim of the docs.
If `writeImage` throws an exception, it won't be caught and rethrown as ImagineException
Now avg_exec_time_us.
Typo'd here & the next line
It seems possible that there won't be any x-dropdowns in the page.   Be very careful with [0].
It could be better using github issues, what you say? @docgnome
`getRepositoryFindOneBy()` doesn't seem to be a clear name for this method. Perhaps `getJohnUsername()`?

Also, consider: `return isset($john) ? $john->getUsername() : null;`.
This new dependency might break on RedHat systems. Due to the fact, that the module is called 'php-devel'.
Could you update this to include the names of the environment variables? Would help along someone new using the script. Also, maybe split this out into three checks so you can be more accurate about what is missing?
If these variables aren't filled out, then the turntable bits would silently not get setup, which would make it tricky to debug if you are setting this up initially.

Could you add an else clause that indicates if any of these are missing?
I’m not sure where else the `DELETE_STAT` is and actually needs to be updated. But what about updating it at a single place only to be on the safe side – I’d suggest to override the `metashare.repository.models.resourceInfoType_model.delete` method.
This should be up on line 217 where the rest of the checks are - not sure why this one got missed though, thanks.
Maybe caching those directories as static variables so that they don't have to be calculated each time?
This won't compile on older versions of Xcode. Not sure if we're going for compatibility or not.
based on indices or boolean mastk
Nice, I changed the code.
Shoulnd't you be getting a compile error here?  You renamed the Constant to REQUIRE instead of USER.
yeah, I don't like the way "MonitorWith" reads here. Find a better name for the annotation? At the very least, the annotations appear to be stating properties of the method, so "MonitoredWith" seems better.
<pre><code>... = ActionRoute $ \req -> maybe (b req) (return .  return) `liftM` a req</code></pre>
Same thing here with the prototypes, I'm not sure this is the best way to show this (though it is the most technically correct). I think it'd make more sense to show an example usage with a note. What do you think?
ngx_http_browser_module模块
This should be updated to ensure the correct value for avatar_updated_at is returned after reloading the model.
Why is this registered on top level?
removed.  now you can either PoisonPill your AMQPActor or shutdown you system.
there should be @ since annotations on every property here, right?
lol

/me has left around some debugging code...nothing new! :P
extraline...
multilines are good
Incidentally, adding a `--` to the end of the `git rev-list` command alters the output to be

    error: No upstream branch found for ''
    error: No upstream branch found for ''
    fatal: bad revision '@{upstream}...HEAD'

Not that this makes any difference, since we're throwing away the error messages anyway, only the success case matters.
Can you factor out this common code between the double and integer case?  Everything except for 

            double[] rankedA = rank(a);
           double[] rankedB = rank(b);

Looks the same, so it'd be nice to have the computation code in one place and reduce the risk of things being wrong.
I see. I wonder why they're doing that, in case there's an actual specific reason for it. Can you please find out?
I wanted to only have the XmlConfigHelper.setValues(this, properties, false, true); call explicitly only where it is needed
I think this should be in the Log interface (probably as 2 different messages).
As you are creating the file content in the test, you should delete it at the end of the testsuite (and not commit it)
Does this always get run? I guess the backends which override this mean that it doesn't?
Please, no trailing backslash continuations.  One way out here would be to use the temporary variable, "fig = self.figure".  That may be the best that can be done short of more substantial refactoring and renaming.  Long names are good--but only up to a point.
Yeah -- I waffled about that.  There is other backend-specific data here in `Matplotlib.nib`, and since this is backend-specific I put it here.  But it's not something I feel too strongly about.
Both lines in this file probably needs to be wrapped.
"une des clef pour bien gérer un projet est de réaliser qu'ecrire du code pour soit meme est une mauvaise idée ...
You probably want to remove that.
I agree with you - but *feel* like it should be ``%{note.title}`` & ``%{note.message}``. Which will give us the flexibility of doing %{note.staff.name} or ``%{note.poster}`` - resolving context issues when compared to ``%{staff.name}`` or even ``%{poster}`` for note poster. See my other comment on the same issue.

That said, it will open a pandora box on the need to add classes for note, message & response objects (which should extend something like class.threaditem.php). You get the idea... I simply chickened out.

I'll implement it _your way_ for now, and then we'll revisit variables post RC3 release. Please? Pretty please??
Agreed, I set headers on a request more often than setting content.
Add an empty line above the function.
With the changes here, if `config[module]` is equal to `null`, then the first check passes and this will be invoked:

`} else if (!feature || config[module][feature] === undefined) {`

In which case I think we end up with a `TypeError` exception, which would not be what we want. Should the value of `null` only be considered valid for the value (i.e., `config[module][feature][element]`).
Can you use a jQuery each here?
Should be `!data.results.length`
Can we improve these selectors a little bit? Having .noscript #noscript is a bit weird.
I'd like to keep this as it was, because it's consistent with content.js. Otherwise, content.js needs to be changed.
It should be getGroupData as well.
They are already removed from search/impl (and therefore from 1.4-SNAPSHOT). See:
https://github.com/sakaiproject/nakamura/pull/817/files
Okay - that's a much cleaner fix to override the `:operation` parameter than to change the data saving semantics entirely for this one widget.
Same here, code (incl. SCSS) is not a chat window ;)
can be changed with:

    if (!file_exists(__DIR__ . '/../vendor')) {
        echo "Missing dependency! Please install all dependencies with composer.";
        echo "<pre>curl -s https://getcomposer.org/installer | php <br/>php composer.phar install</pre>";
        die;
    }
1. if-else statement contains some duplication, ternary operator is simpler. I replace redundant if-else to ?: when it is possible (R# extension highlights such places and suggests to simplify them).
2. I see here just a formatting issue, so i'm going to add linebreaks:
<pre>
return span.Seconds == 1
    ? string.Format(Strings.Get1SecondAgoText(), "1") 
    : string.Format(Strings.GetNSecondsAgoText(), span.Seconds);
</pre>
Will it solve the issue?

Well-formatted ternary operator should always be better from readability point of view.
Compare:
<pre>
<code>
1   if (condition)
2       return x;
3   else
4       return y;

vs

1   return condition ? x : y;
</code>
</pre>
Oauthversion is set in stackmobconfiguration.h with the api keys

- Matt
On Jul 7, 2012 2:55 PM, "Aaron Schlesinger" <
reply@reply.github.com>
wrote:

> > @@ -49,7 +55,7 @@
> >   * Manually configure your session.  Subsequent requests for the
> StackMob
> >   * singleton can use [StackMob stackmob]
> >   */
> > -+ (StackMob *)setApplication:(NSString *)apiKey secret:(NSString
> *)apiSecret appName:(NSString *)appName subDomain:(NSString *)subDomain
> userObjectName:(NSString *)userObjectName apiVersionNumber:(NSNumber
> *)apiVersion;
> > ++ (StackMob *)setApplication:(int)oauthVersion key:(NSString *)apiKey
> secret:(NSString *)apiSecret appName:(NSString *)appName
> subDomain:(NSString *)subDomain userObjectName:(NSString *)userObjectName
> apiVersionNumber:(NSNumber *)apiVersion;
>
> should this take an `OAuthVersion`?
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/stackmob/StackMob_iOS/pull/50/files#r1118687
>
yeah, it's actually supposed to be a "continous series" sent by the worker
Again.. doesn't get_node_text() get the default element again and check length there too?
Might be worth saving this result so you're not calling getElementsByTagName() twice.. though, without profile, I'm not sure how expensive that call is.

Again, Could do this with xpath instead .. something like '/tagname[1]' but I think you'd still get a NodeList so no more efficient than this. So keep using getElementsByTagName()
I'll add a comment: doing the comparison this way deals with 32-bit wrapping.
Please don't use hungarian notation.
Это строка зачем здесь? Не оставляй лишнее
hmm, no.  I've no idea why the formatter did that.  I'm a little surprised it was picked up as a difference - assumed '-w' would be used in the diff.  Happy to revert.
also I'd prefer TITLE being [0] and the permission check (which might be empty) [1]
You could also try and move the assign code into a function:

```
foreach ($quickmod as $option => $qm_ary)
{
    $lang_string = $qm_ary[0];
    $condition = (isset($qm_ary[1])) ? $qm_ary[1] : true;
    phpbb_mcp_add_quickmod($option, $lang_string, $condition);
}
```
Are you sure that you can access the property "DEFAULT_TYPE" as it is declared private in class "JMS\SerializerBundle\Metadata\LinkMetadata"?
The assignment to ``$password`` seems to be dead. Are you sure it is necessary, or maybe you have a typo somewhere?
words:  ' + sum+ ' < 500.');
Created bug- JIRA: https://jira.appcelerator.org/browse/TIMOB-10315
this condition could have been done after the createWindows and only set the zindex for less duplicated code.  Not necessary to change
Sorry, I was thinking the comment applied to the whole section of code. I didn't realize the last line was the one being commented on.
Can we get some space in here and possibly yank out a variable?

```js
var $el = Backbone.$('<' + _.result(this, 'tagName') + '>').attr(attrs);
this.setElement($el, false);
```
you can remove this check, it is already checked in the `shouldSkipValidations`
I agree that it should be internal.

The reason it is not an auto property, is that properties are not valid destinations for the out keyword in C#. I rather have pretty enums than auto properties.
It can be empty and should be most of the time, use a defaultValue.
Rename to `comment_author`.
Why? I am just trimming `open_` here? The end result looks like `gettext('mon')` for example. No formatting is done here.

Deja vu. I am positive you said that is ok at one time here. Should I change it to `gettext(":")` then?
`else if` into new line.
`.boxHeader` -> `.panel .header`?
O no no. Use jQuery for event handling.
+1 on this. The other option is to issue a warning whenever this scenario happens. (so they don't accidentally miss this)
If it won't compile without code signing, why is signing off by default? Will it compile and work without?
Yeah, I wondered about that too, but I think it is quite useful and doesn't add too much complexity, so let's do it :)
Please change this from 'public' to 'abstract' as the class is only extended by classes in the same package.
boom!  like it in this case
While you have hooked up the listener here, it is not removed.  I've found this to be a memory leak issue...
the check style is ignored for this file
We should try to bring this code, as well as the extra code needed for Serf, into a library built from the mod_pagespeed source, rather than duplicating it here.
Assert instead of check & return? This never happens if everything is set up right, right?
Should be `graphics/Graphics.h`; paths are case sensitive on Linux (and I guess Mac?)
I'm not really sure because I don't follow the code well enough yet. If it's a derived estimate then it should just be changed in the objective function and then attached after fitting. I just don't like that there's an attribute set in a method that's changed by other methods. When is it what? It should only be attached when it's set. If it's something that the optimizer is changing then pass it in. If it's derived, attach it after it's done deriving. 

Does that make sense?
yeah lets stick with 2.4.3
Well, i'm not sure what SpoutBlockm actually does to be honest :P I would gladly implement it otherwise :)
It was the only way I could get the PIP requirements to install. It doesn't have anything to do with this patch other than that.
Translatable link doesn't work.
I use spaces too, but the editor (notepad++)'s preferences were set to tabs, I guess that was a problem.
Ah, I didn't take a look to see that the whole file was GPL'd, it's fine in that case.
Nice, embed is so 2012!
I wonder if we should add kwargs here to future proof this?
What about the new version I just pushed?
> ok it seems to be using default ocamlfind directory, so it should work
> fine. Not sure about the non-standrad installation of the C stub, though ...


It’s put on the same directory
Yeah, I favor having examples here, but the chmod 777 does make me rather nervous. When I set up my dev site, I set up my celery and django logs and things to go into my virtualenv container, and everything runs as my user so no permissions needed adjustment. It would be cool if there was a sample for how one would set this up if one were being serious (chmod 755 to the celery user or group or whatever.) But maybe that's not for this, in which case I favor leaving the old lines below the new line, saying:
...anywhere that is writeable. For example: 
....old stuff here...
in discussion "testing 1"
Could've used the youtube partial for this, but this works
I think the stdin arg is useless here.
I would prefer `player` to be an instance of a display object instead of a hash. The views should basically do no calculations or concatenations, they should be handled by the presenter classes.
This doesn't look like a concurrency friendly code. Probably we shouldn't use LoggerWrapper as a singleton anymore. 
And bake some cakes to solve injection problems?
i meant completely get rid of this helper method that simply calls a standard library function :)
FYI: The argument order of assert() methods matters in PHP. In particular the *expected* value always comes before the *actual* value.

It doesn't matter for Python's testing library, but it is good to keep in mind when moving to tests in other languages.
Hmm, you are right! But if the user who launched the console is not local or is not in local group - console should break if `app/cache` and `app/config` are protected.
I have updated the documentation at php.net (will appear in this Friday's build).
I'll put some more info on this into commands.md
no, i just couldn't think of a quick way to make list-of-lists-of... floats or ints that works in n dimensions
I have pushed my change to the github repository and adjust the commit.

Wen
On Jun 26, 2012, at 12:19 PM, Kent Williams wrote:

>> @@ -155,7 +155,7 @@ int main( int argc, char * argv [] )
>> 	  resampleFilter->SetInput (referenceMesh);
>> 	  resampleFilter->SetReferenceMesh (deformedFixedMesh);
>> 	  resampleFilter->Update();
>> -
>> +std::cout<<"stop here!!!!"<<std::endl;
> 
> I think you need to push the branch to your fork, and then submit a new
> pull request, and delete your old pull request.  Is this correct, hans?
> --
> Kent Williams norman-k-williams@uiowa.edu
> 
> 
> 
> 
> 
> 
> On 6/26/12 12:12 PM, "lwen"
> <reply+p-1052885-2fb15493aa24705f008994f249645cb053eabf00-482621@reply.gith
> ub.com> wrote:
> 
>>> @@ -155,7 +155,7 @@ int main( int argc, char * argv [] )
>>>       resampleFilter->SetInput (referenceMesh);
>>>       resampleFilter->SetReferenceMesh (deformedFixedMesh);
>>>       resampleFilter->Update();
>>> -
>>> +std::cout<<"stop here!!!!"<<std::endl;
>> 
>> I made the change. can you see the update?
>> 
>> ---
>> Reply to this email directly or view it on GitHub:
>> https://github.com/BRAINSia/BRAINSStandAlone/pull/87/files#r1052885
> 
> 
> 
> ________________________________
> Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
> ________________________________
> 
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/BRAINSia/BRAINSStandAlone/pull/87/files#r1052939
Organs2 is no longer used, for future reference.
Two "P"'s.
I'd like to move it to some packages instead of inner class approach or if this inner class just makes sense to the Pipeline, maybe it should be private?
can you change this to take the User object, not the UpdateUserRequest object (which will be an internal class once the other change above is made).  You will then wrap this into a UpdateUserRequest object before sending to the ExecuteRESTRequest method.
Ошибки пишутся в System.err
@shacker do we need the quotes around `notificationDate` with the orderBy? Just curious.
@ctweney Hrmm, is there a way to push the accept_encoding into FakableProxy?
It depends on the debugger, yes. Let's avoid idioms that aren't reliable. Make sense?
`mediaServiceBaseUri` and `oAuthUri` have to be validated that they are valid URIs
Updated the code. There was a wrong locator for search results title. Thanks
This shouldn't be in the page object. Also, can we format this with keys on new lines so it's easier to read?
this hides the fail if the locator is not available
Definitely not for this PR, but yes, something to worry about in a future one.
We can't change the above code as it is directly extracted from Firefox (just a couple of changes).
`pl.ylim(ymin=0., ymax=1.)` as dependence is bounded by 0. and 1. if I understand correctly.
@amueller Alright I did not understand that. A comment explaining this would be welcomed :)
2 spaces between 'both' and 'positive'.
Shouldn't we reimplement width property for tableviewrow?
Why `N_PROP_LAST`? Seems like it should be `N_BUFFER_PROP_LAST`
I think AbsoluteLayout should be used, while creating the container,
(for the android target, I have created window, and made sure that the layout is absolute, so that you can define Left, Top etc. coordinates for each element)
Are all logs errors? Can we have log flags where some logs go to stderr, some go to stdout, and some are stored in a file? This would be useful for testing release versions of the software where stderr and stdout would not be visible.
not platforms, they currently say `On BlackBerry: NYI`
Probably no need in this declaration. Comes within NativeControlObject.h
QObject::connect will not take ownership, it only connects the signals to slots.
Next line tests ```profile.user``` to be nil, not sure, if I want to test it 2 times....
Is this class really useful ? I think the whole PR for DoctrineORM can be removed. We should leave the older elements for a while in order not to break BC, but I can't see the point of those elemetns in DoctrineORM.
No, this only removes the file from the super. The previous semantic:

* If you remove a file from a super, you MUST remove it physically

was wrong. This has fixed it.
I think they're not allowed to be added to a super in the first place, but this code seems to assume they can.

Foreign files will be refactored once we get URI integrated with the DFS.
This should work with the URI object, and not return a string (see e.g. Scheme or Hostname route).
add something about the headXxx() helpers after 'declaring variables'.
Absolutissimo, given I use JsonSerializable and I'm lazy to refactor my code to be 5.3 compliant, I've decided to continue with 5.4.
Perhaps then now would be a good time rename the project and separate from vanilla Whiskerboard. There are too many things with the same name without us having adding "this is Whiskerboard, but not quite _that_ Whiskerboard. We use Mongo and had a major structure change."
Since this is rST, use `` to indicate literal text.
`first` returns `nil` for empty sequences, so the `if` is not needed.
I wonder if we can persuade Jason to accept these in Qt 4.8. They're so useful, it feels a crime to keep copying them everywhere.
just added the following ternary at function start

```javascript
percent > 100 ? 100 : percent;
```
About that, what about using `Entity\Bundle`'s fully qualified namespace name in the use statement? I know it's not the case right now, and I quickly checked the code base, and there are 5 uses of the `...\Entity` namespace vs 9 uses of the FQNN. I'd like to homogenize that by always using the FQNN, any opinion on that? (same in `FeatureContext.php`)
can you add a new line at the end of the file
This way we can merge those 3 locators into 1 and remove the remaining unused one too so net gain of fewer locators :)

If the tests covered this area more then we'd probably use a page region here but with the current level of test coverage it's a bit overkill.
Added header to class.
This pre-dates (and was the impetus for) the Sandcastle examples that use the combined Cesium file.  Now that we have the Sandcastle examples, I'll delete this.
I changed the assignment of a GUID to the type if its undefined. Before using a counter to make unique uniform and function names, the type needed to be defined. This is no longer the case, so it can be undefined and now the check for undefined is necessary to make sure it isn't added to the cache.

We need the two conditionals because of `checkForTemplateErrors`.
There is a pretty significant bug here, and below when assigning to `_spPick`.  Do you see it?

.
.
.

When does this get called?  What happens if `clampToPixel` changes?  Does that trigger a recompile to change the `#define`?

However, I don't think we need the `#define`; a uniform should be OK.  The shader could do something like
```
mix(vec2(positionWC.x, positionWC.y), vec2(floor(positionWC.x), floor(positionWC.y)), u_clampToPixel);
```
where `u_clampToPixel` is a floating-point uniform that is `0.0` for false or `1.0` for true.  The GLSL compiler _should_ do a good job of optimizing this.  Even if not, the `floor` calls probably aren't too bad.
How can this ever be true?  You throw above if seconds > 60, and if it's equal to 60 you'll subtract one from it making it 59.
Doesn't the runtime already print exceptions and return with status 1 if they are uncaught?
The constructor should be called only once I think and in other modules we seem to keep initialization here.  Not terribly opposed to changing it
We should probably do a null check for tiView
We may want to provide some sort of feedback that the attribute is protected and not destroyed.

Additionally it may want to exit with 1 as the command did not complete successfully.
/**
 * Declaration and initialization of $GLOBALS['foo'].
 * @global bool $foo
 */
global $foo = false;

/**
 * Function that utilizes some global variables in its body.
 *
 * Notice that these do NOT have a "$varname" piece...
 * it can be helpful to include the $varname in the description text, though.
 *
 * @global bool  uses $foo to decide to do something
 * @global int      uses $bar as how many iterations to run
 * @global array the data to be processed is in $baz
 */
function doSomething() {
    global $foo;             // one var with the 'global' keyword
    global $bar, $baz;  // multiple vars with the one 'global' keyword

    /* do stuff ... */
}
they end up reading the same data either way. Which of the two is the preferred modern syntax?
New caching implementation stores converted html content along with the last mtime of markdown file and regenerates when the markdown file's mtime is changed. Also all the cache files are set to expire after 1 year as the regeneration logic is handled by this method.
Nope, lets leave it consistence with __nativeToBigEndian__

Maybe, it is good idea for another pull request tho