Sample: Use slashes instead of underscores in PHP namespaces for nested enums and messages #2757
Conversation
googlebot
added
the
cla: yes
|
PTAL |
Codecov Report
@@ Coverage Diff @@
## master #2757 +/- ##
============================================
+ Coverage 87.18% 87.19% +<.01%
+ Complexity 5699 5698 -1
============================================
Files 473 473
Lines 22615 22609 -6
Branches 2426 2425 -1
============================================
- Hits 19716 19713 -3
+ Misses 2080 2078 -2
+ Partials 819 818 -1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #2757 +/- ##
============================================
+ Coverage 87.23% 87.25% +0.02%
+ Complexity 5723 5721 -2
============================================
Files 474 474
Lines 22685 22674 -11
Branches 2435 2432 -3
============================================
- Hits 19789 19784 -5
+ Misses 2077 2073 -4
+ Partials 819 817 -2
Continue to review full report at Codecov.
|
|
We don't usually enforce that... but it does seem the changes in this PR could use some more testing. I'll see if I can reach the target code coverage. |
|
PTAL |
|
In this example, the second |
There was a problem hiding this comment.
This looks really nice, thank you @hzyi-google. I think it would also be worthwhile for @michaelbausor to take a peek before merging.
| .append("\\") | ||
| .append(component.substring(0, 1).toUpperCase()) | ||
| .append(component.substring(1)); | ||
| return typeNameConverter.getTypeName(builder.toString()); |
There was a problem hiding this comment.
What is the point of this for loop if we always return on the first element?
|
Fixes #2141 |
|
@michaelbausor Thanks for the catch! Fixed and also added a test. PTAL |
| * </ul> | ||
| * | ||
| * <p>To correctly output these type names, we need to check whether the parent of a proto element | ||
| * is a message, and if so use '_' as a separator. | ||
| */ | ||
| private TypeName getTypeName(ProtoElement elem, int maxDepth) { |
There was a problem hiding this comment.
maxDepth is unused now - can we remove it?
| // Create the type name based on the element's full name | ||
| for (String component : fullName.split("\\.")) { | ||
| builder | ||
| .append("\\") |
There was a problem hiding this comment.
This step of splitting, appending \, capitalizing, and rejoining is duplicated. Consider the following refactor: create a helper function:
private static String makeNamespacePiece(String piece) {
return "\\" + piece.substring(0, 1).toUpperCase() + piece.substring(1);
}Then use streams:
String s = fullName.split("\\.").stream().transform(makeNamespacePiece).joinStrings();(I don't think joinStrings() is a thing, but use a collector to join the strings in some way).
…ed enums and messages (googleapis#2757)
No description provided.