search

json-schema-form / json-schema-builder

The JSON Schema form builder
MIT License
70 stars 16 forks source link

Refactored as a directive #5

Closed ThomasPe closed 8 years ago

ThomasPe commented 8 years ago

I have now refactored the builder controller so it can be used as a directive. I feel it's now at a point where we can start working on it "issues-based" to weed out bugs and add missing features. Please have a look at the code and give me your feedback!

nicklasb commented 8 years ago

Cool. I will take a look! (however there seems to be some conflicts)

ThomasPe commented 8 years ago

I think it's probably just the readme since I haven't synced your latest change.

nicklasb commented 8 years ago

Ok, so I have some comments:

  1. I would like the actual builder application to keep working as before, but using the directive instead. It is important as a demo or service for users, instead add a directive.html that just showcases the directive.
  2. If you want to restructure the source, do not do that now, do that later. Aim for incremental development, a PR should be a well-defined entity, it should be easy for others to understand, merge and roll back. "Refactored as a directive" should only and exactly mean that and nothing more.
  3. I have created a "directive" branch, please rebase and do your PR:s towards that.

Otherwise, good work!

ThomasPe commented 8 years ago

gotcha. I'm having a bit trouble figuring out how to actually rebase to the new branch (Git / GitHub is still rather new to me) - hopefully I can make it work soon.

nicklasb commented 8 years ago

No problems, take it easy. Git is a world of its own.