Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Add package of the week video #150

Merged
merged 2 commits into from
Jan 20, 2021
Merged

Conversation

tvolkert
Copy link
Contributor

No description provided.

@google-cla google-cla bot added the cla: yes label Dec 25, 2020
@tvolkert
Copy link
Contributor Author

@natebosch

@natebosch
Copy link
Contributor

The video doesn't give much information. The video seems like a really good way to help more folks discover the package. After they know it exists and are looking at the docs I don't know what value they would get from a link to the video.

@tvolkert
Copy link
Contributor Author

The package is listed in api.flutter.dev, so they may just click on it to see what it has to offer without any context. For those users, I think the video still provides value.

@natebosch natebosch requested a review from lrhn December 30, 2020 23:21
@natebosch
Copy link
Contributor

cc @lrhn WDYT?

@tvolkert
Copy link
Contributor Author

fwiw, here's what users see right now:

async

@@ -2,6 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

/// {@youtube 560 315 https://www.youtube.com/watch?v=r0tHiCjW2w0}

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a doc comment, so you should probably add a library declaration to make it document the library.

We should also add a real library documentation, but for now, let's just have a headline before the link:

/// Various functionality associated with the asynchronous features of the `dart:async` library.
///
/// {@youtube 560 315 https://www.youtube.com/watch?v=r0tHiCjW2w0}
library pkg.async;

Copy link
Contributor

Choose a reason for hiding this comment

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

you should probably add a library declaration to make it document the library.

Dartdoc should handle it either way - but I'm in favor of the explicit library declaration for now.

I really hope we can make that library; soon.

If I can nitpick the wording, I think "Various" and "functionality" aren't conveying much - virtually any package can be describe as having "Various functionality". How about:

/// Utilities that expand on the asynchronous features of the `dart:async` library.
///
/// {@youtube 560 315 https://www.youtube.com/watch?v=r0tHiCjW2w0}
library pkg.async;

Copy link
Contributor

Choose a reason for hiding this comment

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

That would LGTM

Copy link
Contributor

@natebosch natebosch left a comment

Choose a reason for hiding this comment

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

Adding the doc and link LGTM once it has a header in addition to the link.

@tvolkert
Copy link
Contributor Author

Ok, all done. @natebosch can you merge for me?

@natebosch natebosch merged commit 8e32089 into dart-archive:master Jan 20, 2021
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

3 participants