labofsahil/github-event-dataset-2012 huggingface dataset:
It's required by the upgrade script. Maybe there's another way to do it? I assumed that default only applied during an upgrade but if it affects the table forever it may be worse that allowing the field to be null.

Sent from my iPhone

On Jun 9, 2012, at 10:20 AM, "Nirmal Ranganathan" <reply@reply.github.com> wrote:

>> @@ -25,11 +27,12 @@ def upgrade(migrate_engine):
>> 
>>     # add column:
>>     instances = Table('instances', meta, autoload=True)
>> -    volume_size = Column('volume_size', String(36))
>> -    flavor_id = Column('flavor_id', String(36))
>> -
>> -    instances.create_column(flavor_id)
>> -    instances.create_column(volume_size)
>> +    instances.create_column(Column('flavor_id', String(36), nullable=False,
>> +                            server_default="1"))
>> +    instances.create_column(Column('volume_size', Integer(), nullable=True))
>> +    instances.create_column(Column('tenant_id', String(36), nullable=False,
>> +                            server_default="0"))
> 
> I don't like that we are setting defaults for the flavor_id and tenant_id, but if setting nullable=False requires a default then we'll do it, if not lets remove them, our code to update these values should take care of that, I mean making sure we set those values. 
> 
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/hub-cap/reddwarf_lite/pull/110/files#r955768
This will need some exception handling.
I prefer to see all three of them .
Is this line longer than 79 characters?
doesn't exists doesn't means nobody use this. Who is using "renfoncement" for indentation ?
And... http://fr.wiktionary.org/wiki/indentation
just looks like a really roundabout way to make a matrix. Why not `(to-double-matrix [[row]])` or something?
ok. I am used to give meaningfull names to vars even if it means longer
names.

On Wed, Sep 5, 2012 at 8:37 PM, Richard Rodger <notifications@github.com>wrote:

> In lib/parambulator.js:
>
> > @@ -61,12 +61,32 @@ var quantrule = function( foundok, rulename ) {
> >      }
> >      else return cb();
> >    }
> > -}
> > +};
> > +
> > +var lenrule = function( islenok ) {
> > +  return function(ctxt,cb) {
> > +    var conditionallength = ctxt.rule.spec
>
> rename conditionallength to just len - easier to read
> also, the lenrule is about a "len"
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/rjrodger/parambulator/pull/9/files#r1535858>.
>
>



-- 

*malex*
I really hate this test now :(
Can you switch these around in alphabetical order?
@bp323 Fixed
it's in french
@apex13 Since this is a `<div>` do we need display: block?
@bp323 Can we use single quotes here?
By creating this factory interface I've been able to move all the Query generation code out of the SolrSearchServlet, and encapsulate away all the details of getting information from the SlingHttpServletRequest. A BacwardCompatibleSolrQueryFactory now contains that logic (part of this PR). We can now create other QueryFactory implementations to create reusable queries that are divorced from the Servlet render chain (and classes). For example, SimpleQueryFactory might just generate a Query from a String passed to the constructor, like so:

   SimpleQueryFactory sqf = new SimpleQueryFactory("resourceType:foo");

   Query getFooQuery = sqf.getQuery();
Yeah, that [was my concern too](https://github.com/michalochman/sapphire/commit/03b253c136b59c4ca8b0bfd6b07e8255aa1424b2#commitcomment-1546112).

I didn't know about `TEMP_FOLDER` constant.
If it  must be one result You can use getOneOrNullResult() if You want object ;)
Nit: I would flip the conditions so that the smaller branch is first:

```js
if (help.type !== "cssSelector") {
  helpArea.hide();
  relocator.cleanup();
  return;
}
// rest of code ...
```

I think this increases readability and reduces a nesting level for the main body of code.
Maybe a subdirectory "Ffmpeg" could be in place? This seems like saturating the video plugin space with Ffmpeg plugin's helper?
I suggest rewriting this whole sentence to

> The test modules are contained in the `src/freeseer/test/` directory.
The assignment to ``$feednRenderer`` seems to be dead. Are you sure it is necessary, or maybe you have a typo somewhere?
Are you sure ``''`` has the correct type as "string" is not a sub-type of "object<Zend\Feed\PubSubHubbub\HttpResponse>|object<Zend\Http\PhpEnvironment\Response>"?
fs.existsSync = fs.existsSync || path.existsSync;
Why's this here? Best to remove it entirely if it's not needed to avoid confusion.
copy-pasta from another spec on that same file :) Will cleanup as part of next commit.
Missing optional slash at the end of the path.
Ou remover parágrafo anterior, ou movê-lo para seção seguinte.
"Co-Autores activos pueden administrar la mayor parte de la contribución."
Cool, will remove. Thanks
You say dependency 'puppetlabs/stdlib', '2.x' --- that's wrong, isn't it? I thought dependency statements were supposed to be in 'puppetlabs-stdlib' (dash instead of slash) style. 

(sorry, originally put this on the wrong page.)
Nope, it seems it is now needed by the JMSSecurityExtraBundle. If I remove it, I have the following exception :

> RuntimeException: The JMSSecurityExtraBundle requires the JMSAopBundle, please make sure to enable it in your AppKernel.
Why indenting this one more? One line before is not a block? Should be at the same level.
Use `slideToggle('fast')`. Or in any way, use `fast`.
It works for me, so I suppose it is defined.
Btw: My intention here was strictly to fix the bug, where you would get no facebook image if you havent got facebook username. For Gravatar as you said, there is another ticket.
а шо это тако?
No reason these enums+structs need to be duplicated. Since we control the format, just use XNA ones
Can't hardcode /usr/local (has to be HOMEBREW_PREFIX), the otool and test stuff is unnecessary.
Yes it should be a float.
It gets loaded already with the rest of the standard library above.
>are begin/end for rendering? than i would call these methods: renderBegin and renderEnd

Yeah, you are right. I do that in the next step
Refactored out their need, forgot to remove them - thanks!
Here occurs an error saying: 
function ChatHandler::GetDeletedCharacterInfoList is inaccessible!

maybe due to it being protected for class ChatHandler like the one before.

If anyone can solve this, these scripts are working perfectly.
When you see this pattern, you should probably be using .fetch
Notice that the dictionary is immutable, and never modified. It's cloned, the item added and then replaces the field, so this way millions of readers are not affected by the only 5 writes that will be done when starting up (we have only 5 connection strings).
For this to be usable the constructor needs to take in string values for each attribute.   Should be like :   (string var1, string var2......
Such generic stuff shouldn't be in `resource_style.css`
It looks like that, but no. `rowSpec` defines what a row looks like (number and width of cells). Ditto `colSpec` for columns. If a row has four cells, that's four columns. Thus, `m_numCols(rowSpec.numCells)` and vice-versa.

Yes, this interface is confusing and impossible to memorise. Suggestions welcome :(
Odd use of `size_t` here, and maybe below for `m_minAllocation`.
I'm not very clear on why we can't do w->Disable() after w->SetDisabled(true) here rather than the cast and container check.  I'm guessing their is a reason, event dispatch order, or some such.  I suggest a comment at this point outlining why we have to do it this way.
Hmm. ``__init__.py`` has never been empty in George's branches, but I see now that Josef emptied it out in 85633976f. Since the backwards compatibility break already happened in that commit, I agree we should now be consistent and leave ``__init__.py`` empty.
These two lines can be combined so you'll have: `base_time = options[:base_time] || Time.now`.
We can push these up to final fields, done.
what does fieldname mean here?  Could you write a quick docstring?
i tend to think we should stop allowing a specified URL as it is too fragile.  what if specified URL is https but there is no ssl_config ... what if they contradictory domain_name and specified_url are supplied.

not absolutely sure but seems safer...
See BrooklynSystemProperties for another way...
wrong spacing.  use 4 spaces for tabs.
drupal is the database name within mysql. 

But I can add some documentation.
The assets (js/css) were moved during the start of this SOW. When rebasing commits were automatically applied onto "app/assets/stylesheets/app.scss" however the file currently used on the portal is "app/assets/stylesheets/web/app.scss" so we had to add our CSS changes to this file as well.
`--a` and `a--` are two inherently different operations. `--a` is shorthand for `a -= 1`. `a--` has (relatively) very odd behaviour: `(temp = a; a -= 1; temp)`. When the desired behaviour is "decrement `a`", the simpler instruction should be used to indicate that you don't require that extra, odd behaviour. And even when I do require that behaviour (which isn't often), I prefer to make it explicit if it's not too difficult. So while `a--` is unfortunately more commonly used, I tend to avoid it in almost all cases.
`float /*double*/` should be replaced with `CGFloat`.
`total` here should be an object from the `Presentation` module, not an object.
Fixed in f2a13fbd276d642b866cd8af3ff55f1509d25365.
Fair enough. It is actually already deleted in my local tree, there is no
use for it, that's correct.


On Tue, Dec 11, 2012 at 11:25 AM, peabody124 <notifications@github.com>wrote:

> In ground/openpilotgcs/src/plugins/coreplugin/boardmanager.cpp:
>
> > +        if (!list.contains(vid))
> > +        list.append(vid);
> > +    }
> > +
> > +    return list;
> > +}
> > +
> > +/**
> > + * @brief BoardManager::getKnownBoard
> > + * Returns a known board for a given vendorID and productID
> > + *
> > + * @param vendorID USB vendor ID
> > + * @param productID USB product ID
> > + * @return A board type if the board is known, NULL otherwise
> > + */
> > +IBoardType* BoardManager::getKnownBoard(int vendorID, int productID)
>
> I'd still like to beat this dead horse. There is not a unique mapping from
> VID/PID to IBoardType so this should be deleted. Any usage of it is a bug.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/PhoenixPilot/PhoenixPilot/pull/79/files#r2383127>.
>
>
Would it be better to change the event mechanism to support a "void *" and document this as an opaque pointer?  Then have the code cast it to one or the other on the other side?
Or
Some sort of object hierarchy, once again with most operations ending up with an object that contains a char *, but others with the enhanced data fields.  I don't like this very much, since it ends up being almost the same thing as the other objects, with reinterpret_casts or dynamic_casts there as well.
additional potential test cases, removed for now.
We should change the method's name to something more suggestive, for eg is_app_dev_name_visible
Same as above.. or at least consistent naming? filteredText or something like that?
where does this code come from?
ReactorModel should really be called ReactorManager
`||` instead of `or`
my concern is that although this will work for the integration of this module, that it will not be a pattern that we can universally apply to other modules.

class ntp(
  $servers = ntp::params::servers
) inherits ntp::params {

}

I would prefer that however he enable hiera be consistent across all of our modules if we wind up editing code(maybe that is a misconception on my part?). In the above case, since inheritance is being used it will not be possible to set a param for hiera support.

I hope this clarifies my previous statements.
I would still prefer a more explicit name. People using this parser are not necessarily aware of all the details of the file format and making this scikit-learn code self explanatory by avoiding short variable names and abbreviations is best for learning new stuff by reading the source.

This is the same reason why we are moving away from single letter mathematical variable name even though they are the one used in the literature.
you need to add ``shape="box"`` here.
This is no longer true because of the one-of-K coding in 0b15b04424d2b1b9d7d43ad3a1866d20932588e1; feature names must be at least *convertible to* strings. I suggest we don't make any promises about what is accepted except for strings.
oh, this variable was unused and so it was removed.

But I changed the condition above, to handle 1 as checked and -1 as unchecked, and -2 will be ignored (no coloring).
I would prefer moving 'unless ... ' into the nex line
to ma wyszukac na podstawie cloudow odpowiednia nazwe, w kolejnosci:
NAME-staging
NAME-production
NAME-production1
itd

jak macie zgrabniejszy pomysl to walcie
Ouch, it's my mistake. We should keep the check whether `fd` is really a `pure` function or not.
The way to fix this is basically to remember any partial lines at the end of the read:

```ruby
read_buffer = ''
loop do
  begin
    read_buffer << $stdin.readpartial(4096)
    if line_match = read_buffer.match /(.*\n)(.*)/
      print line_match[1].grep(expression)  # grep complete lines
      read_buffer = line_match[2]           # save remaining partial line for the next iteration
    end
  rescue EOFError
    read_buffer.grep(expression)  # grep any remaining partial line at EOF
    break
  end
end
```
Good question,

That's what the TODO to validate the use of StringObjects is for
You're right, I will change that to generateApiClass
I don't want to wait if lockDali is set. If there is no topology, I guess I should be assuming lockDali is NOT set (where effectively the current code is assuming it IS set)
What do you think needs amending?
cache and allAllocatorsLock need to be...
but I can't see why allAllocators is..
sorry, yes read wrong line.
Yes, I should check for overflow on 
>        n = n * 100 / HTSIZE_LIMIT; // scale up
Using `xfer.hg` on the slave should actually help minimize the diff. No need for e.g.

    -if (MercurialSCM.joinWithPossibleTimeout(slaveHg.unbundle("xfer.hg").pwd(localCache), fromPolling, listener) != 0) {
    +if (MercurialSCM.joinWithPossibleTimeout(slaveHg.unbundle(bundleFileName).pwd(localCache), fromPolling, listener) != 0) {
That could definitely work, will give it a try.
Not sure if this should be delete or deleteLater, or if matters in this context?
nit: missing `;` at end of line.
we dont call cursor.close() here
Well there was definitely a typo here. Do you get the expected result? If so please update the pull request for `lookup_value` and I'll merge it.
I'd like to keep non-user facing paths differentiated somehow. How would you feel about /_/typeahead?
do we really need these ignores
I would; there's no reason for the variable to not have a descriptive name.
and here. Did you mean to do that?
I guess all the setters here work this way, seems like bad practice; we should change it throughout.
I somehow missed @kring and @bagnell comments.  Like I said, I'm okay with bringing this in, I just don't want to pretend like it's okay to not have doc as standard operating procedure, regardless of the age of the branch or how badly people want it in master.  Sounds like @bagnell has some additional changes ready to go.  When those are in, we'll merge away.
Im not sure about this name. How about something like OffsetEvent or CoordinateOffsetEvent?
Should assign to LastSuccessEndTimeInternal.
mobile web supports convertPointToView
Ha! That's a great idea! Thank you, I didn't see it
So... if I'm understanding you correctly, a "Type" can also mean "A keyword" (e.g. "null", "array"), while an FQSEN does not allow those?

If so, this seems like one more reason not to use "Type" here, since a link to "null" or "array" doesn't really make sense.

Or are we talking about "Type" allowing relative identifiers, while FQSEN implies an absolute one?
I guess I don't understand what's changing here, @daniel-pittman.  I thought we were already pulling in the dependencies for the gems listed in the gem.list file (via the "gem fetch" commands that you are removing).  If that is not the case, then is there a flag we can add to those "gem fetch" commands in order to fix this problem instead of adding another layer of indirection (via an additional Ruby script)?  There are two issues that I think I see here (but I may be mistaken, I have to admit I haven't been completely through your new Ruby script line by line, so I might have missed something there).  If I haven't missed something, those issues are (in no particular order):

1. I don't think you are honoring the "reuse previous download" flag if it was set by the user.  If that flag is set when this script is invoked, then you should only download files that you don't already have; if you've got a file already then this script should just use the existing (assumed to be previously downloaded) file instead of downloading another copy.
1. I don't know what the effect on the process of building/booting the Microkernel skipping is if you skip the "mkdir" and "cp -p" commands from lines 259 and 261 of the original file...are you doing that work somewhere else (in your new Ruby script)?

Other than that, I guess I return to the original question...what is this adding (and can the same thing be added without invoking a Ruby script from this shell script)?
дай советiiiк по обоим эвентам. по-моему говно какое-то
во! прям щас – сделай одинаковый курсор обоим кнопкам
мне так больше нравится. юниформный интерфейс – проще будет эти тупые табы заменить на какую-нибудь классную абстракцию
well since all transports extend from base transport .. we wouldn't need to update the implementations .. just add an empty method into base transport.
I think I fixed this by setting the path earlier than here. Can you verify this?
At least `move` should not overwrite `immutable` fields. Consider:

```D
struct S { immutable int a; }
void main() {
    auto obj1 = S(5);
    immutable(int)* p = &obj1.a;
    auto obj2 = move(obj1);
    assert(*p == 5);
}
```

The type `int` could be replaced with any other. The issue here is that the `assert` should succeed by the definition of `immutable`. If `move` took initiative in overwriting `immutable` fields, we'd break immutable.

The structure in this example is still moveable because it's okay to duplicate an `immutable int`. However, in general we should accept there are types that cannot be moved from.
Good suggestion. Check latest commit.
To @9rnsr:
> As a proposal: `TerminalElementOf`?

It looks ambiguous and templated code should be as easy to read as possible. Not sure.
> Not sure what IFTI is. 

Implicit Function Template Instantiation. It's what happens when you use a templated function without explicitly instantiating it. The template arguments are inferred from the function arguments.

> The problem is that since sequence is a template, then the actual type that is passed to sequence is immutable(int[]).

No. It's not. Unless you explicitly instantiate a template with `immutable(T[])`, no template will _ever_ be instantiated with it, because IFTI takes the slice type of the array, not the actual type of the array. So, if you pass an `immutable(T[])` to a templated function, it'll be instantiated with `immutable(T)[]`, not `immutable(T[])`. No templated anything is going to be operating on `immutable` arrays unless it's explicitly instantiated with that type or it's operating on a local variable which was explicitly declared to be fully `immutable`.

How on earth are you getting a fully `immutable` array in a template? Do you have a concrete example of this or you simply assuming that this is a problem based on your reading of the code?
`Calculates the total element count of a multidimensional static array. Considers $(D T) to be an $(D n)-dimensional static array.`
I don't disagree. I just don't want to make a bunch of changes to dummyRanges (especially potentially breaking changes) when I'm working on something that I think will be better. If it's ultimately rejected, then I'll worry about tweaking dummyRanges then.
But why would you do that? The overload won't even work if it's not `bool`...
You mean that you want to check that the slice is the original type exactly? That's not necessarily true with arrays, but as long as the first part still checks for construction and assignment, then the array can't be const or immutable, in which case checking that the slice using `$` is the exact type should work.
Might be better to discuss with Nigel then put a question in the code.
Should we handle signals here as well; eg `SIGTERM`?
separate them? (one test for each component?)
this was the tab ref i saw.
Somewhere, this needs a comment.  Here, constructor, or in the session class is fine.
pls try to document public class; makes it easier for ppl not familar to your code (and reviewing it) to understand it
Porter, 

this is coded correctly - get the open bmr based on pidm... the rounds are stored associated with the id of that round 

----- Original Message -----

From: "nrporter" <notifications@github.com> 
To: "plymouthstate/plymouth-webapp" <plymouth-webapp@noreply.github.com> 
Cc: "bscoleman-psu" <bscoleman@plymouth.edu> 
Sent: Wednesday, October 3, 2012 7:56:26 AM 
Subject: Re: [plymouth-webapp] changes for adding new BM Report App (#425) 


In legacy/hub/HUBSQL.class.php: > + * get the open building manager report for this bm (if they currently have one that is saved and not submitted)
> + * 
> + * @param int $pidm 
> + * @access public
> + * @return string of query
> + *
> + */ 
> +	function getBMReportByPidm($pidm) 
> +	{
> +		$sql = "SELECT * FROM `bm_reports` bmr 
> +						WHERE bmr.pidm = ? 
> +							AND bmr.submitted = 0"; 
> +
> +    if ($data = PSU::db('hub')->GetRow($sql, array($pidm)))
> +    {
> +		  $sql = "SELECT * FROM `bm_rounds` rounds WHERE rounds.bmr_id=? ORDER BY time ASC"; 
Do we want bmr_id=? or bmr_pidm=? here? 
— 
Reply to this email directly or view it on GitHub .
I'm tempted to infer that you're trying to perform an object copy.

Why not `KeyListener.prototype.keybindings = JSON.parse(JSON.stringify(KeyListener.prototype.defaultKeyBindings))` then?
This should be merged with it's "parent" `if()` so you don't need to write `else { $sql = $select_override; }` twice. While you're at it, you can change the operator to `===` ... comparison by type is (yes, insignificantly) faster and `count()` will return an integer, so it's just better. :)

    if ($is_count && count($this->ar_select) === 1)
    {
        ...
    }
It will be moved to gemspec when we have a new version of Warden.
How to differentiate between the two forms? Can we use IDs or different classes?
@schmittjoh The review tool should repect the existing return value when it guesses as subclass of the existing one. ArrayCollection implements Collection so this was right (and it is now wrong as a PersistentCollection could be returned)
Would be better to be `US$ -12,121,212.00`, dont you agree?
`GraphNavigator::accept` either accepts `$type` as an array, or if null, builds it. The type is then passed to the visitor.
So I think it's always an array

It didn't seem to break any test though
delete_doc!/1?
Now that's a good point. Didn't think of that. Let me look at it again.
IMO it is bad practice to mention during entire manual about version, when changes was made...
For this one we have Appendix D.
Also about 'method' word. As a developer I was a bit confused what do you mean :-)
Maybe 'way'?
> perhaps with getComponentName() 

I qualify it: look at this constant in the start of DelayHandler.
> There could be more than one delayer in the same context

I doesn't make sense: I introduce required 'messageGroupId' based on 'id' aatribute. I can qualify 'REQUEST_DATE' header with 'messageGroupId' too.
> Also, what happens if the same message traverses the same delayer twice?

Also doesn't make sense: 'REQUEST_DATE' header is added only before save message into MS and removed in the 'releaseMessage'.
However, I agree: it looks a bit strange & confused, the more so DelayedMessageWrapper has logic only about some indicator, that message was delayed.

Here I have another thought: add 'requestDate' property into DelayedMessageWrapper and store it as 'payload' of new Message in the MS. Then we'll have full encapsulation about delayer logic.
this.lockRegistrySet = true;
Rename to not be 'rand' named
Do we have a support doc link that talks about this?  I would ask dean/david N.  That or briefly explanin why "Rackspace does not support volumes, only ROS, so inputs  volume_size and stripe_count are ignored."
"стандартизированным образом" я бы поставил после информации, чтоб сделать акцент
Please put a space after the //.
@christianv I'm not sure how I feel about having the other 2 labels be the headings, but not this one. It seems a little inconsistent.
you should indent this line now as it is now inside the ``if``
Maybe when/if the use case comes up, we could create a database view `threaded_emails` that consolidates the emails by subject and sender + recipient pairs?
Good point, thanks @jpoyau. 

@wegrata that might be what was happening when we were calling /distribute with a null POST body?  It was certainly causing uwsgi to barf... I wouldn't have thought cross threading that db object would do that, but maybe?
Probably best to try to avoid whitespace changes in order to keep the changeset minimal.
This was a way to share some thoughts about how to group functions / do the split. This needs to be removed before the merge, but looking at the commits, we'll be doing some rebase -i anyway.
Since this `pv` parameter appears to becoming required by more and more endpoints I thought I'd create a single call to manage it. However, I might change this in the future so that `pv` is only fetched once per update process as it's currently adding an additional request for every extension.
Vraie question de ma part : est ce que "accept" n'est pas trop général comme nom de méthode? Un "convertToDTO" ne serait pas plus pertinent? J'imagine que "accept" est une sorte de convention pour dire que l'on ajoute un visitor?
There aren't hard paths in the script, this is just the sample config.  I just updated it slightly to demonstrate the effect of putting the token {CHROM} into the known_vcf setting.
lol @ gemcutter
not sure how this is supposed to work? JNEXT runs on the server (index) and this variable is not defined anywhere
It's to make it easier for reuse if someone overrides `build_analyzer`.
Why `eql?` here and not `==`?
This should be done in the save method of the Tab model.
I think we should use signal/slot to implement this. Send a signal having the triggering node as parameter and maybe plus the the triggered area. Then we can reduce the coupling of the triggered area and the triggering node.
Why group the tests? This is not neccessary.
The interface needs a description.  "An interface for receiving Android lifecycle events."
?  This is not just a wrapper function. It has a different purpose, to take a value out of a hash table and then convert it.
The following must be documented:

`isTransitionAnimation`
`animationDuration`
yes sounds sensible to me, more idiomatic.
Just say 'is not an integer'. Ruby specific docs may throw people.
Fixed, but slightly different wording. Please check.
I defined it as string so that users can write what ever they want - I don't know it it's used usually by issue trackers or only for the planning
I renamed some of these only to match member names with those specified in MSDN article on queue description. I was surprised to find out that the names used there do not match element names in the XML.
Suggest we add another phrase to clarify why - 

This bean is used by the Spring MVC DispatcherServlet to evaluate which URL maps to which inbound endpoint.

(I had to dig around myself while reviewing).

Either that, or refer them to the reference guide for more informatin.
There shouldn't be Author tags in the code.
It's triggered once for each webview. After the callback has been called for each webview it will remove the application  listener and do a notifyRotateComplete(). I will add a comment for the --pendingViewportChanges to avoid any future confusion.
@dkoole @var -> @param
Files start with a capitalized letter.