samples/poppopwin: upgrade to first-class sample

samples/poppopwin: upgrade to first-class sample

No longer a 3rd-party sample. This lives in the SDK

fix 'big' toggle issue
Fixed divide by infinity
Move shared assets to asset dir
removing unneeded JS

R=jmesserly@google.com

Committed: https://code.google.com/p/dart/source/detail?r=34141

[kevmoo, 2014-03-19 00:46:59]

Leveraging pub build a lot...makes things much easier

[Jennifer Messerly, 2014-03-19 01:06:52]

code changes look good BUT, I'm not sure this file is in the right place. We
have (had?) a rule that code under different licenses (e.g. Apache vs BSD) must
be placed under third_party. It gets confusing because the root third_party
folder also sometimes means code we pulled in form elsewhere :|. But we did get
that rule from the open source releasing folks.

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/.gitignore
File samples/pop-pop-win/.gitignore (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/.gitigno...
samples/pop-pop-win/.gitignore:1: packages
I think all you need is "build" here
(packages and pubspec.lock are already in .gitignore for Dart repo)

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/license.txt
File samples/pop-pop-win/license.txt (left):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/license....
samples/pop-pop-win/license.txt:7: http://www.apache.org/licenses/LICENSE-2.0
So, this isn't under the same LICENSE as the rest of the Dart project (which is
BSD)
my understanding is: it should be placed under samples/third_party/...

For example, both "samples/third_party/todomvc" and "pkg/third_party/html5lib"
are developed in Dart SVN. But they are under "third_party" because of the
different license.

(hmm, although looking at TodoMVC now, it uses identical license to Dart SDK, so
maybe we could move it up a level. Our old version used to be under a different
license)

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
File samples/pop-pop-win/pubspec.yaml (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
samples/pop-pop-win/pubspec.yaml:4: homepage:
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/samp...
this URL looks wrong. Or maybe not, depending on how my comment goes with
respect to the LICENSE file ;)

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/web/plat...
File samples/pop-pop-win/web/platform_web.dart (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/web/plat...
samples/pop-pop-win/web/platform_web.dart:77: var loc = window.location;
fwiw, nothing wrong with using "final". It seems to give better inference in the
Editor sometimes. And better VM performance. I do dislike "final TypeName
variableName = ..." tho.

"var" is fine too of course! This is just an FYI comment.

[kevmoo, 2014-03-19 03:24:05]

PTAL

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/.gitignore
File samples/pop-pop-win/.gitignore (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/.gitigno...
samples/pop-pop-win/.gitignore:1: packages
On 2014/03/19 01:06:54, John Messerly wrote:
> I think all you need is "build" here
> (packages and pubspec.lock are already in .gitignore for Dart repo)

I kinda want to leave these here for folks who copy the whole dir to their
system to start development.

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/license.txt
File samples/pop-pop-win/license.txt (left):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/license....
samples/pop-pop-win/license.txt:7: http://www.apache.org/licenses/LICENSE-2.0
On 2014/03/19 01:06:54, John Messerly wrote:
> So, this isn't under the same LICENSE as the rest of the Dart project (which
is
> BSD)
> my understanding is: it should be placed under samples/third_party/...
> 
> For example, both "samples/third_party/todomvc" and "pkg/third_party/html5lib"
> are developed in Dart SVN. But they are under "third_party" because of the
> different license.
> 
> (hmm, although looking at TodoMVC now, it uses identical license to Dart SDK,
so
> maybe we could move it up a level. Our old version used to be under a
different
> license)

Nuking the license file.

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
File samples/pop-pop-win/pubspec.yaml (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
samples/pop-pop-win/pubspec.yaml:4: homepage:
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/samp...
On 2014/03/19 01:06:54, John Messerly wrote:
> this URL looks wrong. Or maybe not, depending on how my comment goes with
> respect to the LICENSE file ;)

Removing the homepage until we get things figured out w/ dartlang.org

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/web/plat...
File samples/pop-pop-win/web/platform_web.dart (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/web/plat...
samples/pop-pop-win/web/platform_web.dart:77: var loc = window.location;
On 2014/03/19 01:06:54, John Messerly wrote:
> fwiw, nothing wrong with using "final". It seems to give better inference in
the
> Editor sometimes. And better VM performance. I do dislike "final TypeName
> variableName = ..." tho.
> 
> "var" is fine too of course! This is just an FYI comment.

Just trying to align w/ our general code guidelines.

[Jennifer Messerly, 2014-03-19 14:26:42]

a few more things I noticed. otherwise lookin good. let me know if I can help
with setting up the automated testing. But it's not very hard to set up.

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/.gitignore
File samples/pop-pop-win/.gitignore (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/.gitigno...
samples/pop-pop-win/.gitignore:1: packages
On 2014/03/19 03:24:06, kevmoo wrote:
> On 2014/03/19 01:06:54, John Messerly wrote:
> > I think all you need is "build" here
> > (packages and pubspec.lock are already in .gitignore for Dart repo)
> 
> I kinda want to leave these here for folks who copy the whole dir to their
> system to start development.

sounds good to me

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/license.txt
File samples/pop-pop-win/license.txt (left):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/license....
samples/pop-pop-win/license.txt:7: http://www.apache.org/licenses/LICENSE-2.0
On 2014/03/19 03:24:06, kevmoo wrote:
> On 2014/03/19 01:06:54, John Messerly wrote:
> > So, this isn't under the same LICENSE as the rest of the Dart project (which
> is
> > BSD)
> > my understanding is: it should be placed under samples/third_party/...
> > 
> > For example, both "samples/third_party/todomvc" and
"pkg/third_party/html5lib"
> > are developed in Dart SVN. But they are under "third_party" because of the
> > different license.
> > 
> > (hmm, although looking at TodoMVC now, it uses identical license to Dart
SDK,
> so
> > maybe we could move it up a level. Our old version used to be under a
> different
> > license)
> 
> Nuking the license file.

Nice. Yeah, since you wrote pop-pop-win it's your call. +1 for having same
license as everything else :)

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
File samples/pop-pop-win/pubspec.yaml (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
samples/pop-pop-win/pubspec.yaml:1: name: poppopwin
I just noticed: the pkg name doesn't match the directory name. Is this about
dashes-vs-underscores?

Rename the folder pop_pop_win or poppopwin?

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
samples/pop-pop-win/pubspec.yaml:4: homepage:
https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/samp...
On 2014/03/19 03:24:06, kevmoo wrote:
> On 2014/03/19 01:06:54, John Messerly wrote:
> > this URL looks wrong. Or maybe not, depending on how my comment goes with
> > respect to the LICENSE file ;)
> 
> Removing the homepage until we get things figured out w/ http://dartlang.org

nice. yeah, unless uploading to pub.dartlang.org it probably doens't matter.

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/web/plat...
File samples/pop-pop-win/web/platform_web.dart (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/web/plat...
samples/pop-pop-win/web/platform_web.dart:77: var loc = window.location;
On 2014/03/19 03:24:06, kevmoo wrote:
> On 2014/03/19 01:06:54, John Messerly wrote:
> > fwiw, nothing wrong with using "final". It seems to give better inference in
> the
> > Editor sometimes. And better VM performance. I do dislike "final TypeName
> > variableName = ..." tho.
> > 
> > "var" is fine too of course! This is just an FYI comment.
> 
> Just trying to align w/ our general code guidelines.

makes sense :)

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
File samples/pop-pop-win/test/browser_test_harness.dart (right):

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/browser_test_harness.dart:2: import
'ppw/_ppw_runner.dart';
style nit: we usually don't name files starting with underscore.

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/browser_test_harness.dart:4: main() {
If you name this file *_test.dart, it should get picked up automatically by
buildbots.

Basically anything ending in *_test.dart is automatically picked up. You won't
need the two harnesses.

then you can run it with:


# console vm
./tools/test.py -m release samples/pop-pop-win

# content_shell dart vm
./tools/test.py -m release -r drt samples/pop-pop-win

# dart2js various browsers
./tools/test.py -m release -r drt,ff,chrome -c dart2js samples/pop-pop-win

# analyzer
./tools/test.py -m release -r none -c dartanalyzer samples/pop-pop-win

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/browser_test_harness.dart:7: runppwTests();
should this be "runPpwTests()" ?

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
File samples/pop-pop-win/test/ppw/_ppw_runner.dart (right):

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/ppw/_ppw_runner.dart:1: library ppw_test;
move this file up a level? not sure the "ppw" directory is adding much

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/ppw/_ppw_runner.dart:11: runppwTests() {
per early suggestion, this file could just be "ppw_test.dart" to have it picked
up by test.dart

then this method could be "main" :)

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/ppw/_ppw_runner.dart:12: group('ppw', (){
space between parens and open curly

[kevmoo, 2014-03-19 16:22:58]

Note to self: update the editor build file, too.

https://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/edit...

[kevmoo, 2014-03-19 21:17:00]

PTAL

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
File samples/pop-pop-win/pubspec.yaml (right):

https://codereview.chromium.org/200723008/diff/1/samples/pop-pop-win/pubspec....
samples/pop-pop-win/pubspec.yaml:1: name: poppopwin
On 2014/03/19 14:26:43, John Messerly wrote:
> I just noticed: the pkg name doesn't match the directory name. Is this about
> dashes-vs-underscores?
> 
> Rename the folder pop_pop_win or poppopwin?

Going with proj and dir name of pop_pop_win

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
File samples/pop-pop-win/test/browser_test_harness.dart (right):

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/browser_test_harness.dart:2: import
'ppw/_ppw_runner.dart';
On 2014/03/19 14:26:43, John Messerly wrote:
> style nit: we usually don't name files starting with underscore. 

I'll do another run-through for these later. Just trying to hit the basics now.

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/browser_test_harness.dart:4: main() {
On 2014/03/19 14:26:43, John Messerly wrote:
> If you name this file *_test.dart, it should get picked up automatically by
> buildbots.
> 
> Basically anything ending in *_test.dart is automatically picked up. You won't
> need the two harnesses.
> 
> then you can run it with:
> 
> 
> # console vm
> ./tools/test.py -m release samples/pop-pop-win
> 
> # content_shell dart vm
> ./tools/test.py -m release -r drt samples/pop-pop-win
> 
> # dart2js various browsers
> ./tools/test.py -m release -r drt,ff,chrome -c dart2js samples/pop-pop-win
> 
> # analyzer
> ./tools/test.py -m release -r none -c dartanalyzer samples/pop-pop-win

Again, a good follow-up for the next CL. I don't want to churn more here.

https://codereview.chromium.org/200723008/diff/20001/samples/pop-pop-win/test...
samples/pop-pop-win/test/browser_test_harness.dart:7: runppwTests();
On 2014/03/19 14:26:43, John Messerly wrote:
> should this be "runPpwTests()" ?

Yup. Again, let me get this set of changes through.

[Jennifer Messerly, 2014-03-19 21:22:15]

well, then lgtm for now.

I filed https://code.google.com/p/dart/issues/detail?id=17641 about lack of
automated tests

https://codereview.chromium.org/200723008/diff/60001/editor/tools/features/co...
File
editor/tools/features/com.google.dart.tools.deploy.feature_releng/build_rcp.xml
(left):

https://codereview.chromium.org/200723008/diff/60001/editor/tools/features/co...
editor/tools/features/com.google.dart.tools.deploy.feature_releng/build_rcp.xml:210:
<exclude name="**/test/"/>
do you want any of these ignore lists besides the ones already above?

[kevmoo, 2014-03-19 21:27:41]

Committed patchset #5 manually as r34141 (presubmit successful).