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

Library tour example tests for dart:io, dart:convert, dart:mirrors #445

Merged
merged 4 commits into from
Nov 24, 2017

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Nov 23, 2017

Contributes to #407 and #416, wrapping up all the rest of the library tour examples.

Continuation of #443

Staged at https://dartlang-org-staging-0.firebaseapp.com/guides/libraries/library-tour#dartio---io-for-command-line-apps

@chalin chalin added fix.examples Adds or changes example a.libraries Relating to the Dart standard libraries. test.general Relates to unit, integration, perf testing labels Nov 23, 2017
@chalin chalin added this to the 1.24 milestone Nov 23, 2017
@chalin chalin requested a review from filiph November 23, 2017 21:19
@googlebot googlebot added the cla: yes Contributor has signed the Contributor License Agreement label Nov 23, 2017
@chalin chalin changed the title Library tour example tests for dart:io Library tour example tests for dart:io, dart:convert, dart:mirrors Nov 24, 2017
{% prettify dart %}
var p = new Person('Bob', 'Smith', 42);
InstanceMirror mirror = reflect(p);

mirror.invoke(#greet, ['Shailen']);
mirror.invoke(#greet, ['Sundar']);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI, this change was copied from

// Invoke a method on the object.
mirror.invoke(#greet, ['Sundar']);

});
}{% endprettify %}
}
{% endprettify %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

0xae, 0xc3, 0xb6, 0xc3, 0xb1, 0xc3, 0xa5, 0xc4,
0xbc, 0xc3, 0xae, 0xc5, 0xbe, 0xc3, 0xa5, 0xc5,
0xa3, 0xc3, 0xae, 0xe1, 0xbb, 0x9d, 0xc3, 0xb1
];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to repeat this long array. In the page it is displayed just a few lines above in the previous excerpt.

@chalin chalin force-pushed the chalin-lib-tour-1123 branch 3 times, most recently from 89c9481 to b247239 Compare November 24, 2017 02:15
Copy link
Contributor

@filiph filiph left a comment

Choose a reason for hiding this comment

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

LGTM with minor comments

@@ -1,8 +1,9 @@
// ignore_for_file: unused_element, type_annotate_public_apis
// #docplaster
import 'package:test/test.dart';
// import 'package:test/test.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the changes to this file. Do you want to delete the file instead? Or add a comment explaining why it exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a template file I've been using when creating new test files. I've removed it.


<div class="alert alert-warning" markdown="1">
**Important:**
Only command-line scripts and servers can import and use `dart:io`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think Flutter apps can also import dart:io.
https://docs.flutter.io/flutter/dart-io/dart-io-library.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added "Flutter mobile apps" to the "only" list.

{% prettify dart %}
import 'dart:mirrors';
{% endprettify %}

<div class="alert alert-warning" markdown="1">
**Warning:**
Using dart:mirrors can cause dart2js to generate very large JavaScript
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we are ready to update this section. Maybe keep mirrors & @MirrorsUsed but also add a paragraph about package:build etc. Not necessarily in this PR.

cc @kevmoo to see if we're ready for this. Newcomers to Dart still learn about mirrors before anything else.

@chalin
Copy link
Contributor Author

chalin commented Nov 24, 2017

Thanks for the feedback @filiph

@chalin chalin merged commit e8a4c90 into dart-lang:master Nov 24, 2017
@chalin chalin deleted the chalin-lib-tour-1123 branch November 24, 2017 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a.libraries Relating to the Dart standard libraries. 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.

None yet

3 participants