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

Rewriting the README #48

Merged
merged 19 commits into from
May 15, 2015
Merged

Rewriting the README #48

merged 19 commits into from
May 15, 2015

Conversation

irace
Copy link
Contributor

@irace irace commented May 14, 2015

#32

Rewriting the README to incorporate all of the API changes. I’d absolutely love your feedback on how we can make this more comprehensive yet as approachable and clear as possible.

Some links may be broken until the Carthage PR is merged; I’ll handle that once it’s in.

There’s also a link to a wiki page which I’ve yet to write. This’ll happen tomorrow.

@AriX @mattbischoff @prendio2 @PadraigK

@prendio2
Copy link
Contributor

This looks great. I think I may have missed some of the conversation about API changes because the new API is even simpler and more intuitive than I remembered. Until I saw the new README I wasn't aware additional attachments were able to be created using standard objects like strings and images.

It might be nice to show an example of the item providing blocks in use, the point that they exist is perhaps lost on a quick scan without an example.

Using Instapaper as an example of an app that doesn't accept images, even hypothetically, may be misleading once the next update comes out which addresses that. (Of course apps having to pretend they accept images when there's nothing they can do with them is symptomatic of the whole issue)

Tumblr should be listed in the "Apps that use XExtensionItem" section… We use it in Unread but not sure that's particularly pertinent at the moment unless we start add additional custom parameters for certain extensions in the future.

@mbbischoff
Copy link
Contributor

The Copyright should be 2015.

@mbbischoff
Copy link
Contributor

@{ @"tumblr-custom-url": @"/post/123/best-post-ever" }

Wouldn’t we want them to use public constants for the keys?

@irace
Copy link
Contributor Author

irace commented May 15, 2015

Wouldn’t we want them to use public constants for the keys?

That's removed in this PR, no?

@mbbischoff
Copy link
Contributor

Apparently I’m going colorblind.


# XExtensionItem
<h1 align="center">XExtensionItem</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

<h1 align="center">XExtensionItem</p>

Did you mean <h1 align="center">XExtensionItem</h1>?

@mbbischoff
Copy link
Contributor

Looking really good, but I feel like I might already know too much to read this with fresh eyes. Maybe @khanlou, @cdzombak, @grantjbutler, or @bcapps would have more useful first-read feedback. https://github.com/tumblr/XExtensionItem/blob/feature/readme/README.md

@irace irace added this to the 1.0 milestone May 15, 2015
@irace
Copy link
Contributor Author

irace commented May 15, 2015

This looks great. I think I may have missed some of the conversation about API changes because the new API is even simpler and more intuitive than I remembered. Until I saw the new README I wasn't aware additional attachments were able to be created using standard objects like strings and images.

Yes, @AriX made these changes. They’re great!

It might be nice to show an example of the item providing blocks in use, the point that they exist is perhaps lost on a quick scan without an example.

Sure thing, I’ll add one.

Using Instapaper as an example of an app that doesn't accept images, even hypothetically, may be misleading once the next update comes out which addresses that. (Of course apps having to pretend they accept images when there's nothing they can do with them is symptomatic of the whole issue).

Good call, I’ll choose a different example.

Tumblr should be listed in the "Apps that use XExtensionItem" section… We use it in Unread but not sure that's particularly pertinent at the moment unless we start add additional custom parameters for certain extensions in the future.

Yep, totally. I’ll make separate sections for apps that use it vs. extensions that use it. I’d like to list Unread even though it doesn’t have any custom parameter support.

@irace
Copy link
Contributor Author

irace commented May 15, 2015

I’m going to merge this, but I’d still love any feedback that anyone has!

irace added a commit that referenced this pull request May 15, 2015
@irace irace merged commit d60af19 into master May 15, 2015
@irace irace deleted the feature/readme branch May 15, 2015 19:19
@AriX
Copy link
Contributor

AriX commented May 17, 2015

README looks great. The one thought I had when reading through it is that (especially in the "Why?" section) you mostly talk about the usefulness of XExtensionItemSource, but not about the greater aspirations of allowing apps to pass richer metadata to each other and custom parameters and such. Just wanted to point that out!

@irace
Copy link
Contributor Author

irace commented May 18, 2015

Thanks @AriX – I’ve elaborated a bit to hopefully address your feedback: f0307c2

@AriX
Copy link
Contributor

AriX commented May 18, 2015

Sweet!

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.

None yet

4 participants