RFC: Size Munger -> Prow "Size" Plugin

[nlandolfi]

I propose writing the size munger as a Prow plugin named size.

Objective

Write a Prow plugin implementing the same functionality as the size munger. This functionality is defined in Background.

Background

The size munger applies labels to pull-requests based on their size. The size can be one of XS, S, M, L, XL, XXL. A PR is sized based on the number of relevant lines in the diff it induces with the base branch.

We define relevant lines as those in non-generated files. The set of generated files is defined in a configuration file in the target repository.

Generated Files Config

A sample generated files configuration is available here. We briefly define the spec of this file.

The config is a series of newline-delimited statements. Statements that begin with a # are ignored. A statement is a white-space delimited key-value tuple.

statement = key val

where whitespace is ignored, and:

key = "path" | "file-name" | "path-prefix" | 
      "file-prefix" | "paths-from-repo"

For example:

# Simple generated files config

file-prefix	zz_generated.

# Line with `#` ignored.
file-name	generated.pb.go
file-name	generated.proto

# ...

The statement's key specifies the type of the corresponding value:

Type	Description
path	exact path to a single file
file-name	exact leaf file name, regardless of path
path-prefix	prefix match on the file path
file-prefix	prefix match of the leaf filename (no path)
paths-from-repo	load file paths from a file in repo

(this table taken from here)

PR Sizing

Once the relevant files are determined (i.e., lines in generated files ignored as specified in the config), we count the number of relevant additions and deletions in the diff induced in the PR. We call this count l.

Size	Corresponding l
XS	[0, 10)
S	[10, 30)
M	[30, 100)
L	[100, 500)
XL	[500, 1000)
XXL	[1000, )

Visibility on GitHub

The size assigned to a PR will be made visible on github by labelling the PR in the form size/<size> (e.g., "size/L").

Implementation Proposal

1. Add a directory to kubernetes/test-infra/prow/plugins named size. Directory contains code for a Prow plugin named "size".
2. Register a plugins.PullRequestHandler
3. When any PR has action "opened" or "synchronize" perform calculations by reading file from repo; update size label if needed.

Details

Configuring name of generated files config in repo

Update: First implementation will assume the file is called .generated_files and forego any additional configuration modifications.

Add a configuration Size to the prow config struct.

(@spxtr is this the way to go? plugins seemed to be listed in plugins.yaml but configured in config.yaml?)

// Modify this type:

type Config struct { // see prow/config/config.go
    // existing ...
    Size Size `json:"size,omitempty"`
}

// Add these types: 

type Size struct{
    Repos map[string]SizeRepo `json:"repos,omitempty"`
}

type SizeRepo {
    ConfigFile string `json:"config_file,omitempty"`
}

corresponding to a block in a Prow config.yaml:

size:
  repos:
    kubernetes/kubernetes:
        config_file: ".generated_files"

Which revision of config file?

Consider the kubernetes/kubernetes config above. Suppose the user mutates the .generated_files file. Adding:

file-name ignoreme.100lines

Their PR then included the new file ignoreme.100lines which contained 100 new lines.

We will use the .generated_files located at the base branch revision (in the usual case this is the file currently in the master branch). Therefore this PR would have 101 lines of relevant diffs, and be labeled size/L. head branch revision, therefore this PR would have 1 line of relevant diffs and be labeled size/XS.

[spxtr]

Nice write-up.

Configuring name of generated files config in repo

I don't think we even need the config option. Is there any problem with just assuming that the file is called .generated_files?

Which revision of config file?

I suggest using the version from the PR, although the file changes infrequently enough that it's not a big deal.

[0xmichalis]

/area prow
/area mungegithub

[nlandolfi]

Is there any problem with just assuming that the file is called .generated_files?

I guess I figure allowing the configuration option with a sane default is more future-proof. If the file name should ever need to change in the future or be different per repo...not sure how big of a deal that is (it's probably cause I've spent some time re-parameterizing certain prow config assumptions 😛) but this assumption leads to a simpler implementation so I'm cool with it.

Which revision of config file?

So then in the above example, the change would be 1 line and considered XS, correct? That makes sense to me.

i'm updating the main text accordingly

[spxtr]

Yep that sounds right.

[spxtr]

One thing to note is that other plugins want the .generated_files parsing logic. In particular, I want to skip running golint on generated files. If you could design your code such that this logic is easy to break out then I would be very happy :)

[nlandolfi]

Definitely. I designed the .generated_files business apart from the sizing logic, more details in the companion PR I just opened here