Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analyze/test cmdline tutorial code excerpts #1902

Merged
merged 4 commits into from
Sep 19, 2019

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Sep 15, 2019

Resolves first item of #1803. (Builds on #1900, #1901.)

Staged at https://dart-dev-staging-0.firebaseapp.com/tutorials/server/cmdline
Original https://dart.dev/tutorials/server/cmdline


  • Added dcat.dart example, which comes from https://github.com/dart-lang/dart-tutorials-samples/tree/master/cmdline/bin (we should consider removing it from that repo).
  • Created other examples sources (not found under dart-tutorials-samples, etc.) as necessary.
  • Updated cmdline.md page links to ensure that they all reference the proper API version (based on the channel). Also ensured that all links to external sites consistently open in a new tab. (We aren't doing that elsewhere, but at least the links in this page will be consistent in their behavior).

Note: dcat, and the rest of the newly added code, is analyzed but NOT tested; we don't test other command line apps in this repo -- doing so would require some refactoring or additional recripting, which probably isn't worth it at this point.

I've ensured that all external links in the page are valid:

~ > blc -fgi http://localhost:4000/tutorials/server/cmdline
Getting links from: http://localhost:4000/tutorials/server/cmdline
├───OK─── http://www.google.com/intl/en/policies/privacy/
├───OK─── https://dartpad.dev/
├───OK─── https://pub.dev/
├───OK─── https://dart-lang.github.io/server
├───OK─── https://flutter.dev/
├───OK─── https://pub.dev/packages/args
├───OK─── http://creativecommons.org/licenses/by/3.0/
├───OK─── https://api.dart.dev/
├───OK─── https://github.com/dart-lang/site-www/tree/master/src/_tutorials/server/cmdline.md
├───OK─── https://pub.dev/documentation/args/latest/args/ArgParser-class.html
├───OK─── https://medium.com/dartlang/announcing-dart-2-5-super-charged-development-328822024970
├───OK─── https://github.com/dart-lang/site-www/issues/new?...
├───OK─── https://api.dart.dev/stable/dart-async/Future-class.html
├───OK─── https://medium.com/dartlang
├───OK─── https://pub.dev/documentation/args/latest/args/ArgResults-class.html
├───OK─── https://github.com/dart-lang/site-www
├───OK─── https://pub.dev/documentation/args/latest/
├───OK─── https://api.dart.dev/stable/dart-async/Stream-class.html
├───OK─── https://github.com/dart-lang/site-www/issues
├───OK─── https://api.dart.dev/stable/dart-io/stdout.html
├───OK─── https://api.dart.dev/stable/dart-io/stderr.html
├───OK─── https://api.dart.dev/stable/dart-io/stdin.html
├───OK─── https://api.dart.dev/stable/dart-io/FileSystemEntity-class.html
├───OK─── https://api.dart.dev/stable/dart-io/File-class.html
├───OK─── https://api.dart.dev/stable/dart-io/Platform-class.html
├───OK─── https://api.dart.dev/stable/dart-io/Platform/environment.html
├───OK─── https://api.dart.dev/stable/dart-io/Platform/isMacOS.html
├───OK─── https://api.dart.dev/stable/dart-io/Platform/numberOfProcessors.html
├───OK─── https://api.dart.dev/stable/dart-io/Platform/script.html
├───OK─── https://api.dart.dev/stable/dart-io/IOSink-class.html
├───OK─── https://api.dart.dev/stable/dart-io/Directory-class.html
├───OK─── https://api.dart.dev/stable/dart-io/exitCode.html
├───OK─── https://api.dart.dev/stable/dart-io/exit.html
├───OK─── https://api.dart.dev/stable/dart-io/dart-io-library.html

@chalin chalin added a.tut.tutorial Relates to the Tutorial section of dart.dev fix.examples Adds or changes example test.general Relates to unit, integration, perf testing labels Sep 15, 2019
@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Sep 15, 2019
@chalin
Copy link
Contributor Author

chalin commented Sep 15, 2019

@kwalrath: I've tried to stage my changes but I can't seem to get that to work (even if I re-login, and update to the latest firebase-tools).

Edit: for the record, I had to logout before re-logging in; then deployment worked once again.

Copy link
Contributor Author

@chalin chalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes for @kwalrath.

src/_tutorials/server/cmdline.md Outdated Show resolved Hide resolved
src/_tutorials/server/cmdline.md Show resolved Hide resolved
src/_tutorials/server/cmdline.md Show resolved Hide resolved
src/_tutorials/server/cmdline.md Show resolved Hide resolved
@chalin chalin mentioned this pull request Sep 15, 2019
23 tasks
@chalin chalin force-pushed the chalin-cmdline-dcat-0911m branch 3 times, most recently from 28bbe61 to eae6375 Compare September 16, 2019 22:02
@chalin
Copy link
Contributor Author

chalin commented Sep 17, 2019

Staged at https://dart-dev-staging-0.firebaseapp.com/tutorials/server/cmdline (link added to opening comment).

@chalin chalin force-pushed the chalin-cmdline-dcat-0911m branch 2 times, most recently from 0b37f10 to 9bf8417 Compare September 17, 2019 13:17
src/_tutorials/server/cmdline.md Outdated Show resolved Hide resolved
src/_tutorials/server/cmdline.md Show resolved Hide resolved
src/_tutorials/server/cmdline.md Outdated Show resolved Hide resolved
src/_tutorials/server/cmdline.md Outdated Show resolved Hide resolved
src/_tutorials/server/cmdline.md Outdated Show resolved Hide resolved
src/_tutorials/server/cmdline.md Outdated Show resolved Hide resolved
src/_tutorials/server/cmdline.md Outdated Show resolved Hide resolved
src/_tutorials/server/cmdline.md Show resolved Hide resolved
src/_tutorials/server/cmdline.md Outdated Show resolved Hide resolved
src/_tutorials/server/cmdline.md Show resolved Hide resolved
Copy link
Contributor

@kwalrath kwalrath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like most of your changes very much. Most of my comments are of the nitpicky variety, and some of them don't involve your changes. But since we're in there...

@chalin
Copy link
Contributor Author

chalin commented Sep 18, 2019

General question: should references to dart:foo packages be in backticks in the text? I my first pass over the page I chose not to make that change. The list in Some specific items to put in code font would seem to suggest that dart:foo should be in backticks. What's your call?

@chalin
Copy link
Contributor Author

chalin commented Sep 18, 2019

Updates done and staging server refreshed. I've added a few questions as comments. (External links rechecked ✔️)

@kwalrath
Copy link
Contributor

Just curious... What's up with the date in the footer? Is your system time off?

image

Copy link
Contributor

@kwalrath kwalrath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kwalrath
Copy link
Contributor

We should probably have a DPE check the code... :/

@kwalrath
Copy link
Contributor

General question: should references to dart:foo packages be in backticks in the text? I my first pass over the page I chose not to make that change. The list in Some specific items to put in code font would seem to suggest that dart:foo should be in backticks. What's your call?

I'm inclined to avoid using backticks for them, although I suspect backticks have been creeping in. Here's my thinking:

  • Libraries aren't mentioned by name in the style guide.
  • There's no way that dart:something can be confused for anything but a core library.
  • We have so much code font in the docs already, and it's quite distracting.

But I'm not convinced I'm right. :) What do you think, @chalin?

@chalin
Copy link
Contributor Author

chalin commented Sep 18, 2019

Just curious... What's up with the date in the footer? Is your system time off?

No it's because when I build for a PR, I used a fixed "site generation" date. That makes it easier to do full-generated-site file diffs (which I do regularly).

Here is the command line I use:

$ ./tool/serve.sh --pin-now --dev

@chalin
Copy link
Contributor Author

chalin commented Sep 18, 2019

I'm inclined to avoid using backticks ...

Let's go with your preference for now. Thanks for the feedback.

@chalin
Copy link
Contributor Author

chalin commented Sep 19, 2019

@johnpryan - do you think that you'll be able review this today? It should be quick to review. The dcat code was copied from https://github.com/dart-lang/dart-tutorials-samples/tree/master/cmdline/bin. Thanks.

Copy link
Contributor

@johnpryan johnpryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just a minor nitpick.

examples/misc/pubspec.yaml Outdated Show resolved Hide resolved
@kwalrath kwalrath merged commit ef37e66 into dart-lang:master Sep 19, 2019
@chalin chalin deleted the chalin-cmdline-dcat-0911m branch September 19, 2019 17:11
chalin added a commit that referenced this pull request Sep 24, 2019
Contributes to #1803.
Followup to #1902.

* Move cat_no_hash.dart from lib to bin

Fixes #1927

* Add a basic test for each of the three "cat" commands

* Ensure that there are no errors when dcat or dcat1 are run
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a.tut.tutorial Relates to the Tutorial section of dart.dev cla: yes Contributor has signed the Contributor License Agreement fix.examples Adds or changes example test.general Relates to unit, integration, perf testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants