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

add better naming for api controller methods #391

Closed
wants to merge 4 commits into from

Conversation

vaibhavmule
Copy link
Contributor

reference:

Phoenix Framework

user_path  GET     /users           HelloWeb.UserController :index
user_path  GET     /users/:id/edit  HelloWeb.UserController :edit
user_path  GET     /users/new       HelloWeb.UserController :new
user_path  GET     /users/:id       HelloWeb.UserController :show
user_path  POST    /users           HelloWeb.UserController :create
user_path  PATCH   /users/:id       HelloWeb.UserController :update
           PUT     /users/:id       HelloWeb.UserController :update
user_path  DELETE  /users/:id       HelloWeb.UserController :delete

Rails:

GET	/photos	photos#index	display a list of all photos
GET	/photos/new	photos#new	return an HTML form for creating a new photo
POST	/photos	photos#create	create a new photo
GET	/photos/:id	photos#show	display a specific photo
GET	/photos/:id/edit	photos#edit	return an HTML form for editing a photo
PATCH/PUT	/photos/:id	photos#update	update a specific photo
DELETE	/photos/:id	photos#destroy	delete a specific photo

reference:

Phoenix Framework
```
user_path  GET     /users           HelloWeb.UserController :index
user_path  GET     /users/:id/edit  HelloWeb.UserController :edit
user_path  GET     /users/new       HelloWeb.UserController :new
user_path  GET     /users/:id       HelloWeb.UserController :show
user_path  POST    /users           HelloWeb.UserController :create
user_path  PATCH   /users/:id       HelloWeb.UserController :update
           PUT     /users/:id       HelloWeb.UserController :update
user_path  DELETE  /users/:id       HelloWeb.UserController :delete
```

Rails:
```
GET	/photos	photos#index	display a list of all photos
GET	/photos/new	photos#new	return an HTML form for creating a new photo
POST	/photos	photos#create	create a new photo
GET	/photos/:id	photos#show	display a specific photo
GET	/photos/:id/edit	photos#edit	return an HTML form for editing a photo
PATCH/PUT	/photos/:id	photos#update	update a specific photo
DELETE	/photos/:id	photos#destroy	delete a specific photo
```
@vaibhavmule vaibhavmule requested review from josephmancuso and a team October 1, 2018 06:04
@coveralls
Copy link

coveralls commented Oct 1, 2018

Coverage Status

Coverage remained the same at 91.702% when pulling 51d931f on vaibhavmule-patch-1 into 0207b42 on develop.

@clsource
Copy link

clsource commented Oct 1, 2018

Maybe different templates could be used to emulate masonite, phoenix and rails ways.

@josephmancuso
Copy link
Member

I keep looking at this PR but what is "better" about these?

@vaibhavmule
Copy link
Contributor Author

I'm not sure about the better, but the developer coming from MVC based framework background might find it confsuing,
When you say in show is showing all resources and index is show single resource.

We can keep store but then I wanted to follow CRUD naming. so changed store to create and destroy to delete

@josephmancuso
Copy link
Member

🤔 hmm ok I think i'm on board now but I'm not a huge fan of new and create. They would sound the same to me

@vaibhavmule
Copy link
Contributor Author

Yeah, you are right. They do sound similar and not convey what it does, create I can understand, but yeah, create vs new is confusing to me too.

@josephmancuso josephmancuso added this to Needs review in Masonite Beta 3 Oct 12, 2018
@josephmancuso
Copy link
Member

I'm gonna close this until we are in more of an agreement on what to call the methods

Masonite Beta 3 automation moved this from Needs review to Done Oct 17, 2018
@vaibhavmule
Copy link
Contributor Author

sure

@josephmancuso josephmancuso deleted the vaibhavmule-patch-1 branch October 18, 2018 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants