Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Oct 18, 2016

To reduce the overhead, we should improve these two async tasks as well. @ScottIsAFool trained my eye to spot such things 👍

@dnfclas
Copy link

dnfclas commented Oct 18, 2016

Hi @jlnostr, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@deltakosh
Copy link
Contributor

There is a build error

/// <typeparam name="T">Type of object saved</typeparam>
/// <param name="filePath">Path to the file that will contain the object</param>
/// <param name="value">Object to save</param>
/// <returns>Waiting task until completion</returns>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe something clearer in the comment w.r.t return task?

Copy link
Author

Choose a reason for hiding this comment

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

What do you think about Returns the <see cref="StorageFile"/> that the content is saved to.?

Copy link
Contributor

Choose a reason for hiding this comment

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

Following the style here I think this might do
When this method completes, it returns the <see cref="StorageFile"/> where the Object was saved.

@Odonno
Copy link
Contributor

Odonno commented Oct 18, 2016

Looks good.

Good suggestion @hermitdave 👍
@bartlannoeye
Copy link
Contributor

LGTM

@ScottIsAFool ScottIsAFool merged commit ac4096a into CommunityToolkit:dev Oct 18, 2016
@bartlannoeye bartlannoeye mentioned this pull request Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants