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

Clarity for API Description namespace #1

Merged
merged 13 commits into from
Aug 3, 2015

Conversation

smizell
Copy link
Contributor

@smizell smizell commented Jun 18, 2015

@smizell
Copy link
Contributor Author

smizell commented Jun 18, 2015

@zdne I've updated the doc, FYI.

## Combine Namespaces

We should combine the API Description namespace with the resource namespace.
Right now they are separate and it would be clearer if they were not.
Copy link
Member

Choose a reason for hiding this comment

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

👍

@zdne
Copy link
Member

zdne commented Jun 19, 2015

Done with the review. Please see my comments.

Legend:

  • 👍 Yes. Let's do it.
  • 👎 No, I am against this.
  • 👌 I am easy on this one.
  • ❔ Need more clarification to understand.

@smizell
Copy link
Contributor Author

smizell commented Jun 19, 2015

@zdne I've address some comments. If you can check those out, let me know, and maybe we can move forward.

@zdne
Copy link
Member

zdne commented Jun 30, 2015

@smizell

I've address some comments

I am not sure if you wanted to push any changes to this doc (if so a commit is missing?) or by "address" you simply mean you have add your comments on my comments.

In any case, I've replied to your comments on my comments 😆

@pksunkara
Copy link
Contributor

I have reviewed and commented on with my opinions.

@zdne
Copy link
Member

zdne commented Jul 21, 2015

@smizell what is the status of this ?

@smizell
Copy link
Contributor Author

smizell commented Jul 21, 2015

@zdne I have unfortunately had no time to address this.

@zdne
Copy link
Member

zdne commented Jul 21, 2015

@smizell OK but it is still open and we are totally doing it, right? 👯

@smizell
Copy link
Contributor Author

smizell commented Jul 21, 2015

@zdne Yes! I want to do it, and think it needs to be done.


## Subclass Asset

We should subclass the Asset Element and add one called “messageShema” and another called “messageBody.”
Copy link
Member

Choose a reason for hiding this comment

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

Typo: messageShema

Remove "Elements in Attributes" from API description clarity
@smizell
Copy link
Contributor Author

smizell commented Jul 28, 2015

@danielgtaylor I've added a few comments throughout.

@kylef Relying on you to address these comments through some PR to the Refract namespaces. Thanks for doing that (in advance)!

Remove "Subclass Asset" from API description clarity
@zdne
Copy link
Member

zdne commented Jul 31, 2015

To me this PR is almost ready to go. The only really unresolved point seems to be the #1 (comment)

The rest can be addressed later by introducing a non-breaking change. I would focus mainly on what we need now and make sure we won't have to break it tomorrow (e.g. any future changes we see happening will be non-breaking).

Thoughts?

@smizell
Copy link
Contributor Author

smizell commented Jul 31, 2015

@zdne I agree. I think we should:

  • Remove changes we'll address later
  • Create other RFCs outlining those documents so we can discuss in further detail.

I think Kyle has already done some of the first.

smizell added a commit that referenced this pull request Aug 3, 2015
…tion

Clarity for API Description namespace
@smizell smizell merged commit e15fe3f into master Aug 3, 2015
@smizell smizell deleted the smizell/clarity-api-description branch August 3, 2015 17:23
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.

5 participants