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

RFC for moving some elements to utility profile #31

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

smizell
Copy link
Contributor

@smizell smizell commented Dec 3, 2015

No description provided.

# Motivation

The main motivation is to make the base specification as simple as possible. In
ever implementation we've created for consuming Refract, such as from the Parse
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor typo

In ever implementation...

In every implementation.

@zdne
Copy link
Member

zdne commented Dec 4, 2015

The main motivation is to make the base specification as simple as possible. In ever implementation we've created for consuming Refract, such as from the Parse

I am not sure this is the reason refract is complex ...

@zdne
Copy link
Member

zdne commented Dec 4, 2015

Furthermore I would love to solve inheritance and mixins as we have solved to do so (and that is why DS namespace has to have its own https://github.com/refractproject/refract-spec/blob/master/namespaces/data-structure-namespace.md#inheritance-and-expanded-element

@smizell
Copy link
Contributor Author

smizell commented Dec 4, 2015

I am not sure this is the reason refract is complex

Totally agree. But this reduces complexity.

Furthermore I would love to solve inheritance and mixins

Let's do it :)


The main motivation is to make the base specification as simple as possible. In
ever implementation we've created for consuming Refract, such as from the Parse
Result or API Description results, we have not needed to use any of these
Copy link
Member

Choose a reason for hiding this comment

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

This is false statement. We have used in in the Data Structure namespace and since API Description and Parse Result are building on it they are using these elements (albeit indirectly).

Copy link
Member

Choose a reason for hiding this comment

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

From https://github.com/refractproject/refract-spec/blob/master/namespaces/api-description-namespace.md#api-description-namespace :

This document extends Refract Data Structure Namespace to define REST Resource data structure elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note I mention here that we have no needed the values in "consuming" because these elements do not show up. We also have not needed them in the Fury adapter. So while we do need them in this particular namespace, my point is that:

  1. You could remove them from the base spec
  2. Build a library that conforms to the base spec
  3. Still be able to build/consume the structures we're using

The point was to move these out of the spec so that a library conforming to Refract would be smaller, yet still be able to get the job done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BUT I'm all for leaving in the spec as well. That's really the topic of discussion, as to whether they are needed for every possible profile/namespace. If we think so, let's close this RFC.

Note: you @zdne have mentioned we should address inheritance in the base spec, and if so, these element may become more important. So far, we have left the inheritance up to the Data Structure spec, which is really the place these elements are needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I will have to support @zdne here. We are using extend, select and option extensively in Data Structure namespace.

@zdne
Copy link
Member

zdne commented Dec 8, 2015

@smizell I am not saying the elements being subject of this RFC should remain in the base spec. But if the new namespace is for the data structure namespace (hereafter DSN) only (at this point) I see it as very unfortunate because the way extended is defined caused so many problems in the DSN.

Ideally, what I would suggest is:

  • remove extend, select and option from the base spec
  • add select, option to the DSN
  • revisit DSN inheritance and the use extend element (which, I am afraid is too late to do now)

@zdne
Copy link
Member

zdne commented Dec 8, 2015

@smizell in the meantime, to provide a solution for the simplification of base spec, lets move extend, select and option to the DSN? It will hurt the DSN but will hopefully make the refract more accessible? (I do not see moving those elements into a new namespace as a better solution simply because 1. nobody else is using them, 2. I would like that potential namespace to address inheritance and fix extend)

This proposes we move `extend`, `select`, and `option` from the base
specification to its own profile.

Note that I'm calling this a "profile" versus a "namespace." This is pending
Copy link
Contributor

Choose a reason for hiding this comment

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

Since that RFC is merged, please change this paragraph.

@smizell
Copy link
Contributor Author

smizell commented Jan 14, 2016

@zdne This works for me IF AND ONLY IF this is best for reducing complexity of the base spec while retaining all necessary functionality of data structures.

And I would vote we revisit inheritance ASAP. I've created an issue for this.

@pksunkara
Copy link
Contributor

@smizell @zdne What is our path forward on this?

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.

4 participants