Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

TODOs on the Client side #1260

Open
lirantal opened this issue Mar 10, 2016 · 6 comments
Open

TODOs on the Client side #1260

lirantal opened this issue Mar 10, 2016 · 6 comments

Comments

@lirantal
Copy link
Member

We have a few TODOs that were added, let's see if they make sense or clean them up:

@mleanos would you like to take a stab at this?

@lirantal lirantal added this to the 0.5.0 milestone Mar 10, 2016
@mleanos
Copy link
Member

mleanos commented Mar 11, 2016

I've been thinking about this. Both TODO's have the potential to be big changes.

  1. Articles client-controller: May need a big refactor of the service. This could be a first phase of exploring client-side models.
  2. Users Admin client-service: This could require some back-end router changes; similar to @trainerbill's suggestion of using Express.router in each route configuration.

I'll look into both of these to see if I can come up with a good solution to each, and how much work/refactoring is involved.

@lirantal
Copy link
Member Author

Ok so it's either we decide to fullfill the TODOs or we remove them from the code.
@ilanbiala , @codydaig to review if the TODOs are really necessary or not.

@ilanbiala
Copy link
Member

Both seem valid, and I think they could just be a quick refactor. @mleanos for the first we could just move the code, right? For the second, it might be a bit more refactoring in other places where we use this, so I'm not sure how the dependencies would work out, but if they stay clean across modules then it shouldn't be an issue.

mleanos added a commit to mleanos/mean that referenced this issue Apr 28, 2016
Extends the ArticlesService $resource object to include a custom method
for creating, or updating, an Article instance.

Related meanjs#1260
mleanos added a commit that referenced this issue Jul 11, 2016
Extends the ArticlesService $resource object to include a custom method
for creating, or updating, an Article instance.

Related #1260
@lirantal
Copy link
Member Author

One issue we tackled and merged.
Remaining issues?

@mleanos
Copy link
Member

mleanos commented Oct 12, 2016

@lirantal This TODO can wait until after we release 0.5.0. This change will be a bit involved, and I'm still not sure how we want to go about it.

@lirantal
Copy link
Member Author

Sure

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants