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

DocBlocks for services don't use absolute namespaces #65

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

SimantovYousoufov
Copy link

Preface
I have not added any tests for this PR because I was unsure how to actually run the test suite. If someone can provide some insight into how to run the suite, I'll add them in whatever tests are required ASAP.

Issue
These examples use code from the gRPC example.

Currently, the generated docblocks for services return:

    /**
     * @param routeguide\Point $input
     */
    public function GetFeature(\routeguide\Point $argument, $metadata = array(), $options = array()) {
      return $this->_simpleRequest('/routeguide.RouteGuide/GetFeature', $argument, '\routeguide\Feature::deserialize', $metadata, $options);
    }

These doc blocks are

  1. Not using absolute namespacing, breaking the typehints in IDEs.
  2. Not accurately documenting input params and their types ($metadata and $options).

Proposal
Add logic to DrSlump\Protobuf\Compiler\PhpGenerator@compileStub to handle the above cases based on the RPC type.

It will now return:

    /**
     * @param \routeguide\Point $argument
     * @return \routeguide\Feature
     */
    public function GetFeature(\routeguide\Point $argument, $metadata = array(), $options = array()) {
      return $this->_simpleRequest('/routeguide.RouteGuide/GetFeature', $argument, '\routeguide\Feature::deserialize', $metadata, $options);
    }
    /**
     * @param \routeguide\Rectangle $argument
     * @param array $metadata
     * @param array $options
     * @return \Grpc\ServerStreamingCall
     */
    public function ListFeatures($argument, $metadata = array(), $options = array()) {
      return $this->_serverStreamRequest('/routeguide.RouteGuide/ListFeatures', $argument, '\routeguide\Feature::deserialize', $metadata, $options);
    }
    /**
     * @param array $metadata
     * @param array $options
     * @return \Grpc\ClientStreamingCall
     */
    public function RecordRoute($metadata = array(), $options = array()) {
      return $this->_clientStreamRequest('/routeguide.RouteGuide/RecordRoute', '\routeguide\RouteSummary::deserialize', $metadata, $options);
    }
    /**
     * @param array $metadata
     * @param array $options
     * @return \Grpc\BidiStreamingCall
     */
    public function RouteChat($metadata = array(), $options = array()) {
      return $this->_bidiRequest('/routeguide.RouteGuide/RouteChat', '\routeguide\RouteNote::deserialize', $metadata, $options);
    }

I think the logic for how docblocks and actual method code are generated can be rewritten to generate them in parallel and merge them into $s after generation, but I'm hesitant to make these changes without knowing how to use the test suite and the maintainer(s) desires.

fuhry and others added 18 commits April 16, 2014 16:05
…_basestub

codegen: have each generated client class extends Grpc\BaseStub class
…all_calls

Make sure call options are passed to all types of calls
Update package name and homepage
Rename conflicting function signature in PHP7
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.

5 participants