m-lab / etl

M-Lab ingestion pipeline
Apache License 2.0
22 stars 7 forks source link

Add schema for pcap index #1009

Closed cristinaleonr closed 3 years ago

cristinaleonr commented 3 years ago

This change is Reviewable

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 6556


Changes Missing Coverage Covered Lines Changed/Added Lines %
schema/pcap.go 9 11 81.82%
<!-- Total: 10 12 83.33% -->
Files with Coverage Reduction New Missed Lines %
active/active.go 2 89.89%
<!-- Total: 2 -->
Totals Coverage Status
Change from base Build 6545: 0.008%
Covered Lines: 3515
Relevant Lines: 5591

💛 - Coveralls
cristinaleonr commented 3 years ago

schema/pcap.go, line 9 at r1 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…
Please add a simple docstring for this type. Does VS Code report "golint" warning? (In my interface it's a yellow curvy line under the type name).

Added, thanks!

I don't see a warning. I will look into why that is.

cristinaleonr commented 3 years ago

schema/descriptions/PCAPRow.yaml, line 1 at r1 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…
Hmm. It looks like all the fields used in the PCAPRow struct are defined now in the toplevel.yaml ... I think we can keep this file as a reminder, but I think it can be empty without losing any behavior we need.

Makes sense. I deleted the file contents.

cristinaleonr commented 3 years ago

schema/pcap.go, line 9 at r3 (raw file):

Previously, stephen-soltesz (Stephen Soltesz) wrote…
nit: please end Go comments with a period. Paraphrasing from Effective Go, > Doc comments work best as complete sentences, ...

Done, thanks!