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

Modularize into separate gems #813

Merged
merged 26 commits into from
Aug 3, 2016

Conversation

blowmage
Copy link
Contributor

@blowmage blowmage commented Aug 1, 2016

Separate the monolithic package into individual packages by service.

[closes #341]

This gem will be shared across all google-cloud gems.
Remove old tasks and config files, ensure that glcoud and google-cloud gems run tests.
Revise coverage tasks, allow for gem tasks to be run individually and as a whole.
We are now relying on the googleapis-common-protos gem for these files.
Add tasks to build the jsondoc reports.
Update travis task to run all the jsondoc reports.
This task is much slower that just running all tests at once.
Useful for finding problems in syntax or docs though, watch the output.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 1, 2016
@coveralls
Copy link

coveralls commented Aug 1, 2016

Coverage Status

Coverage decreased (-2.8%) to 92.178% when pulling bc6d0dd on blowmage:individual-packages into 96322e9 on GoogleCloudPlatform:master.

@blowmage
Copy link
Contributor Author

blowmage commented Aug 1, 2016

The coveralls decrease makes sense. We are switching from measuring coverage with unit tests and acceptance tests to just unit tests. Also, we have introduced many new files and some have lower coverage.

@quartzmo
Copy link
Member

quartzmo commented Aug 1, 2016

@blowmage Thanks for explaining that. It shouldn't be hard to increase unit test coverage to a passing level, correct?

@quartzmo
Copy link
Member

quartzmo commented Aug 1, 2016

@blowmage Do you intend to continue supporting the rake console task? I don't see it.

@blowmage
Copy link
Contributor Author

blowmage commented Aug 2, 2016

Each gem has a console task, but there isn't one for the top level yet. If there were one, it should just load the google-cloud gem, and let the service gems get loaded when called.

@coveralls
Copy link

coveralls commented Aug 2, 2016

Coverage Status

Coverage decreased (-2.8%) to 92.178% when pulling 7ffaf3c on blowmage:individual-packages into 96322e9 on GoogleCloudPlatform:master.

Allow the bundle update/install step to be skipped, speeding up the tasks.
Pass in an optional argument to force the bundle update.

    $ rake test:each[update]
Rename Backoff to GrpcBackoff, since this is now private.
Fix calls to Core::GCE.
@coveralls
Copy link

coveralls commented Aug 2, 2016

Coverage Status

Coverage decreased (-1.6%) to 93.353% when pulling 2c7ddd8 on blowmage:individual-packages into 96322e9 on GoogleCloudPlatform:master.

@coveralls
Copy link

coveralls commented Aug 2, 2016

Coverage Status

Coverage decreased (-0.7%) to 94.301% when pulling eaa1a296fc4e735c179cdc24a6ebe5db55774dc6 on blowmage:individual-packages into 96322e9 on GoogleCloudPlatform:master.

Users should convert annotation objects to arrays and hashes explicitly, not implicitly.
Add test coverage for explict conversion methods.
Remove private and unused Image#url method.
Extract the rescue to a method, similar to the one used in GRPC service classes.
This change increases total code coverage.
@coveralls
Copy link

coveralls commented Aug 2, 2016

Coverage Status

Coverage increased (+1.0%) to 95.955% when pulling 6bd18fb on blowmage:individual-packages into 96322e9 on GoogleCloudPlatform:master.

@quartzmo
Copy link
Member

quartzmo commented Aug 2, 2016

I have a feeling we will need to change how docs json is generated. From GoogleCloudPlatform/gcloud-common#157, it appears that in addition to generating the docs json for the individual service packages (which we currently support), we will also have to generate a "bundled package" docs json containing all of the packages.

@blowmage
Copy link
Contributor Author

blowmage commented Aug 2, 2016

I'm happy with the state of the individual packages. There is some additional work around updating the documentation site and automating releases, but I think this PR is ready to be accepted.

@blowmage
Copy link
Contributor Author

blowmage commented Aug 2, 2016

I don't know that we will need to change the way the JSON docs are generated as much as add to it. We'll need the JSON files for the individual packages either way, right?

I have some reservations about merging all the docs into one "bundled" package. Viewers of the docs won't be shown which package the classes and methods are defined in, and more importantly which version of the package they are defined in. I would prefer to see more linking across packages than duplicating the documentation into the "bundled" package.

Resource Manager does not call out to Google::Cloud::Core::GCE, so no need to
stub the call in the test. Was causing an error in the test.
@quartzmo
Copy link
Member

quartzmo commented Aug 3, 2016

@blowmage I would really like to merge this now, so that I can better assist with the effort. Can you create issues now for the outstanding tasks, such as docs json generation?

@blowmage
Copy link
Contributor Author

blowmage commented Aug 3, 2016

Yep! Will add them as soon as this is accepted.

@quartzmo
Copy link
Member

quartzmo commented Aug 3, 2016

OK, I will merge it now.

@coveralls
Copy link

coveralls commented Aug 3, 2016

Coverage Status

Coverage increased (+1.0%) to 95.955% when pulling 861ae4f on blowmage:individual-packages into 96322e9 on GoogleCloudPlatform:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement. packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should we publish a separate module (gem) for each Google Cloud service?
4 participants