Skip to content

Make bigquery functional classes extend info classes - #609

Merged
aozarov merged 5 commits into
googleapis:masterfrom
mziccard:bigquery-hierachies
Feb 4, 2016
Merged

Make bigquery functional classes extend info classes#609
aozarov merged 5 commits into
googleapis:masterfrom
mziccard:bigquery-hierachies

Conversation

@mziccard

@mziccardmziccard commented Feb 2, 2016

Copy link
Copy Markdown
Contributor

This PR performs the following changes

  • Make Dataset extend DatasetInfo
  • Make Table extend TableInfo
  • Make Job extend JobInfo
  • Update READMEs and snippets
  • Update BigQueryExample
  • Update and add more tests

@mziccard
Make bigquery functional classes extend info classes
b0c0bb7
- Dataset extends DatasetInfo
- Table extends TableInfo
- Job extends JobInfo
- Update READMEs and snippets
- Update BigQueryExample
- Update and add more tests
@mziccardmziccard added the api: bigquery Issues related to the BigQuery API. label Feb 2, 2016
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 2, 2016
@ajkannan

ajkannan commented Feb 3, 2016

Copy link
Copy Markdown

Since the functional objects should now be serializable, we should add them to SerializationTest

ajkannan
ajkannan reviewed Feb 3, 2016
View reviewed changes
Comment threadgcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/Dataset.java Outdated

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@ajkannan

ajkannan commented Feb 3, 2016

Copy link
Copy Markdown

@mziccard took a first pass, looks good! Just some minor comments from me.

aozarov
aozarov reviewed Feb 3, 2016
View reviewed changes
Comment threadgcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/DatasetInfo.java Outdated

This comment was marked as spam.

Sign in to view

@mziccard
Minor refactoring bigquery functional objects
94533b2
- Make Builder public
- Make Dataset.builder package scope
- Add toAndFromPb test
- Remove static get methods
@mziccard
Add functional classes to SerializationTest
5829d81
@mziccard

mziccard commented Feb 3, 2016

Copy link
Copy Markdown
ContributorAuthor

Comments addressed

@ajkannan

ajkannan commented Feb 3, 2016

Copy link
Copy Markdown

Looks good, one comment from me about removing the functional class builder() methods entirely. I also think we should manually test the snippets in the READMEs/package-info to make sure that there aren't any access issues, missing imports, or typos.

I'll take care of comments that referenced Resource Manager (ie removing the package scope builder() methods from Resource Manager's Project class and making sure any Project.staticMethods() are removed as appropriate).

aozarov
aozarov reviewed Feb 4, 2016
View reviewed changes
Comment threadgcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/JobInfo.java Outdated
private final String userEmail;
private final JobConfiguration configuration;
/**
* Base class for a {@code JobInfo} builder.

This comment was marked as spam.

Sign in to view

@aozarov

aozarov commented Feb 4, 2016

Copy link
Copy Markdown
Contributor

Looks great! I really like this change.
Few nits and suggestions.
Also, unrelated:

Missing some @Override annotation
WriteChannelConfiguration
TableDataWriteChannel
QueryJobConfiguration
LoadJobConfiguration
ExtractJobConfiguration
CopyJobConfiguration
BigQueryImpl

Missing servialVersionUID
Acl.View

@ajkannanajkannan mentioned this pull request Feb 4, 2016
Merged
@mziccard
Refactor bigquery function objects builders
b6ad451
- Remove static builder methods
- Add builder methods params to builder constructors
- Better javadoc for builder classes
@mziccard
Add missing override annotation and serialVersionUID
fbcbfb4
@mziccard

mziccard commented Feb 4, 2016

Copy link
Copy Markdown
ContributorAuthor

Comments fixed and opened #618

aozarov added a commit that referenced this pull request Feb 4, 2016
@aozarov
Merge pull request#609from mziccard/bigquery-hierachies
4bf3796
Make bigquery functional classes extend info classes
@aozarov
aozarov merged commit 4bf3796 into googleapis:masterFeb 4, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

api: bigqueryIssues related to the BigQuery API.cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mziccard@ajkannan@aozarov@googlebot