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

Rename Table methods in todo #34

Merged
merged 1 commit into from
Oct 15, 2016
Merged

Rename Table methods in todo #34

merged 1 commit into from
Oct 15, 2016

Conversation

harikt
Copy link
Contributor

@harikt harikt commented Oct 15, 2016

Hi Paul,

I was trying to pick up some of the todo stuffs.

Not sure if this is really you are looking for :

Rename Table::insert(), update(), delete() to insertRow(), updateRow(), deleteRow(), and rename newInsert() to insert() , newUpdate() to update(), newDelete() to delete()

I wonder whether the insert() , update(), delete() etc need to be made public. But I am still not sure how they are executed if the Query objects are exposed.

Probably this may not be the PR you are looking for :-) .

…), deleteRow(), and rename newInsert() to insert() , newUpdate() to update(), newDelete() to delete()
@harikt
Copy link
Contributor Author

harikt commented Oct 15, 2016

May be you are looking to add class similar to MapperSelect ? So the queries are exposed and can be executed ?

@pmjones
Copy link
Contributor

pmjones commented Oct 15, 2016

@harikt This is a good start, thanks. Yes, the idea will be make insert() et al public so that you can "reach down" into the lower levels to do lower-level stuff.

@pmjones pmjones merged commit 17a475c into atlasphp:1.x Oct 15, 2016
@pmjones
Copy link
Contributor

pmjones commented Oct 15, 2016

Thanks for jump-starting this @harikt -- I have built on your work and pushed the result to 1.x.

@harikt harikt deleted the table branch October 15, 2016 18:19
@harikt
Copy link
Contributor Author

harikt commented Oct 15, 2016

Good to know Paul.

I would have worked some more, but was not sure where I was heading to :) .

Have a nice day ahead.

Enjoy! .

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.

None yet

2 participants