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

BigQuery code generator #1342

Merged
merged 1 commit into from
Aug 11, 2017
Merged

Conversation

jskeet
Copy link
Collaborator

@jskeet jskeet commented Aug 10, 2017

The code generator code itself is reasonably ugly, but that's not too much of a concern.
The second commit shows an example with four methods for label modification.
If we like this tool in general, I'll probably go back and apply it to the subset of existing methods that an easily be templated.

@codecov
Copy link

codecov bot commented Aug 10, 2017

Codecov Report

Merging #1342 into master will decrease coverage by 0.13%.
The diff coverage is 26.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1342      +/-   ##
==========================================
- Coverage   61.79%   61.66%   -0.14%     
==========================================
  Files         292      294       +2     
  Lines        7882     7912      +30     
==========================================
+ Hits         4871     4879       +8     
- Misses       3011     3033      +22
Impacted Files Coverage Δ
...V2/Google.Cloud.BigQuery.V2/ModifyLabelsOptions.cs 0% <0%> (ø)
....Cloud.BigQuery.V2/BigQueryClient.DatasetLabels.cs 33.33% <33.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd9b138...36b7856. Read the comment docs.

@jskeet
Copy link
Collaborator Author

jskeet commented Aug 10, 2017

(If this looks good, I'll only merge the first commit, and put the rest in another PR that has the implementation as well.)

@jskeet
Copy link
Collaborator Author

jskeet commented Aug 11, 2017

Overnight I've worked out some ways I can make the code much cleaner. Should do exactly the same thing though... feel free to hold off reviewing until I've pinged this thread again.

@jskeet
Copy link
Collaborator Author

jskeet commented Aug 11, 2017

Okay, refactoring complete. Now most of the code works in terms of Parameter objects, which allows a lot more uniformity.

@jskeet
Copy link
Collaborator Author

jskeet commented Aug 11, 2017

The solution file changes are due to dotnet/project-system#1821

Copy link
Contributor

@chrisdunelm chrisdunelm left a comment

Choose a reason for hiding this comment

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

Crude, but better than copy/pasting :)
LGTM

// Methods modifying labels on datasets

#region SetDatasetLabel
/// <summary>

This comment was marked as spam.

This comment was marked as spam.

}
}

private static IEnumerable<string> ModifyCode(ApiMethod method, IEnumerable<string> existingCode)

This comment was marked as spam.

This comment was marked as spam.

Still a work in progress, but could massively reduce the scope for copy/paste typos.
@jskeet jskeet force-pushed the overload-generator branch 2 times, most recently from 8d0c035 to 36b7856 Compare August 11, 2017 10:20
@jskeet
Copy link
Collaborator Author

jskeet commented Aug 11, 2017

(Reduced to one commit, so no actual generated code. Will merge on green, then start working on the labels feature...)

@jskeet jskeet merged commit 7dc3a7a into googleapis:master Aug 11, 2017
@jskeet jskeet deleted the overload-generator branch August 11, 2017 11:01
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