Automatic tag table generation #1950
Conversation
|
|
||
| public void generate() { | ||
| Path path = this.projectRoot.toPath(); | ||
| Pattern pattern = Pattern.compile(this.inclusionPattern); |
There was a problem hiding this comment.
I don't think anyone would call this method multiple times on the same instance but what do you think about doing this in the ctor so that the class would look like:
private final File projectRoot;
private final Pattern pattern;
private final File outputDir;
Printing out the pattern in the next line should be ok, the toString of a Pattern is the string representation of it (inclusionPattern).
| Files.write(output, stringBuilder.toString().getBytes()); | ||
| } | ||
| catch (IOException e) { | ||
| throw new IllegalStateException(e); |
There was a problem hiding this comment.
I guess IOE is thrown from walkFileTree or from write, would not it be better to thrown an IllegalArgumentException (since the class is not in an illegal state) or throwing up the IOException?
| /** | ||
| * @return allowed tag keys | ||
| */ | ||
| default TagKey[] getTagKeys() { |
There was a problem hiding this comment.
SpanEntry has
Collection<KeyValueEntry> tagKeys;
Collection<KeyValueEntry> events;Shouldn't they be consistent?
There was a problem hiding this comment.
Not really cause SpanEntry is just a pojo that I'm using in the table. It's much easier to work with TagKey[] when enums in their .values() method return an array.
| private static final Pattern SPECIAL_REGEX_CHARS = Pattern.compile("[{}()\\[\\].+*?^$\\\\|]"); | ||
|
|
||
| private DocumentedSpanAssertions() { | ||
| throw new IllegalStateException("Can't instantiate utility class"); |
There was a problem hiding this comment.
UnsupportedOperationException?
There was a problem hiding this comment.
I have those ISE everywhere else in such cases so I'll leave this one here too.
| * @param tags array of tags | ||
| * @return a merged array of tags | ||
| */ | ||
| static TagKey[] merge(TagKey[]... tags) { |
There was a problem hiding this comment.
This is basically concat, right? If so I think I would rename it, also, if I understand this correctly, this should do the same in a simpler way:
return Arrays.stream(tags)
.flatMap(Arrays::stream)
.toArray(TagKey[]::new);
|
Done via d2041cb |
Without this change we don't even really know how many tags are created and what are their values.
With this change we want this information to be automatically rendered.