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.

Sign up for GitHub

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

Jump to bottom

Cleaning-up adapter methods and information #582

Merged
merged 2 commits into from
Jul 30, 2016
Merged

Cleaning-up adapter methods and information #582

merged 2 commits into from
Jul 30, 2016

Conversation

sstok
Copy link
Contributor

@sstok sstok commented Jul 30, 2016

Q A
Fixed tickets
License MIT

sstok
sstok reviewed Jul 30, 2016
View reviewed changes
src/Adapter/Adapter.php
*
* @throws AdapterException when state is unsupported
* @internal param int $page
Copy link
Contributor Author

@sstok sstok Jul 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

PHPStorm!! could you please stop doing this 💀 😠 I removed the parameter, that means it's gone. SO THERE IS NO NEED TO ADD A USELESS @internal phpdoc...

@sstok sstok merged commit 053abbe into gushphp:master Jul 30, 2016
sstok added a commit that referenced this pull request Jul 30, 2016
@sstok
remove #582 Cleaning-up adapter methods and information (sstok)
fe0e763 
This PR was merged into the master branch.

Discussion
----------

|Q            |A  |
|---          |---|
|Fixed tickets|   |
|License      |MIT|

Commits
-------

fb4c7bf Remove old unused methods
053abbe Make pagination limit-only (no page indication)
@sstok sstok deleted the cleaning branch July 30, 2016 10:02
@cordoval
Copy link
Member

cordoval commented Jul 30, 2016 •
edited
Loading

why we don't need the merge method? Gush used to be faster at just letting the cloud merge the PR. I am thinking the current command was modified that way to do it all locally but the option to also merge on the cloud should have been perhaps kept. I am not so happy about this.

@cordoval cordoval mentioned this pull request Jul 30, 2016
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sstok @cordoval