Conversation
- Add support for known jira keys for issue link detection - Update dependencies - Use `go fix` to modernize
|
I can apparently merge without a review, but will wait for approvals. |
| // key. If key is too long for the description to fit the GitHub limit, a | ||
| // generic placeholder is used instead. | ||
| func jiraMissingCommitDescription(key string) string { | ||
| if s := fmt.Sprintf(missingCommitExplanationJira, key); len(s) <= maxStatusDescriptionLen { |
There was a problem hiding this comment.
This seems like overkill, if we're going to fall back to "KEY" anyway, we should probably just do that. (Listing out all the available options that might be worthwhile, but for a single example this seems excessive)
| // handling: "[EDGE-12]" matches on the word boundary. The keys must already | ||
| // have been validated by parseJiraProjectKeys. | ||
| func compileJiraKeyRE(keys []string) *regexp.Regexp { | ||
| return regexp.MustCompile(`\b(?:` + strings.Join(keys, "|") + `)-\d+\b`) |
There was a problem hiding this comment.
For extra paranoia, we should probably regexp.QuoteMeta each of them.
| // Requiring an explicit allowlist keeps a bare "PROJ-123" shape from matching | ||
| // incidental text like "Fixes UTF-8 handling" or "Fixes RFC-2119". | ||
| func parseJiraProjectKeys(spec string) ([]string, error) { | ||
| var keys []string |
There was a problem hiding this comment.
I recommend
| var keys []string | |
| if spec == "" { | |
| return nil, nil | |
| } | |
| var keys []string |
and then reject an empty string below. Rationale: Other than "" itself, the only way we get an empty string is if someone mangles the flag value, and that should probably be an error rather than a silent skip.
| } | ||
|
|
||
| // jiraProjectKeyRE matches a well-formed Jira project key. | ||
| var jiraProjectKeyRE = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9]*$`) |
There was a problem hiding this comment.
Do we want to allow lowercase? It seems like the lexical convention is ALL UPPER.
| // plausible Jira project key (letters and digits, starting with a letter). | ||
| // Requiring an explicit allowlist keeps a bare "PROJ-123" shape from matching | ||
| // incidental text like "Fixes UTF-8 handling" or "Fixes RFC-2119". | ||
| func parseJiraProjectKeys(spec string) ([]string, error) { |
There was a problem hiding this comment.
Consider maybe folding the parse and compile together?
func parseJiraProjectKeys(spec string) (*regexp.Regexp, error) {since the caller is just going to compile them anyway, and nil means skip.
| } | ||
|
|
||
| func TestParseJiraProjectKeys(t *testing.T) { | ||
| t.Run("Empty", func(t *testing.T) { |
There was a problem hiding this comment.
I think we should reject this entirely.
|
(also: typo in |
mikeodr
changed the title
3 participants
go fixto modernize