Add flag to specify filename other than CODENOTIFY (#15)

- Add a `-filename` option to the codenotify tool, with a default value of `"CODENOTIFY"`. - Modify the Markdown output slightly to account for the possibility of different file names. - Add an input to the GitHub workflow, which allows users of the workflow to pass in a different value for the filename.
sourcegraph · Jan 31, 2022 · 1a31067 · 1a31067
1 parent 515c85d
commit 1a31067
Show file tree

Hide file tree

Showing 3 changed files with 109 additions and 48 deletions.
diff --git a/action.yml b/action.yml
@@ -1,5 +1,10 @@
 name: 'codenotify'
 description: 'Notify subscribed users and groups of changes.'
+inputs:
+  filename:
+    description: 'Filename in which file subscribers are defined'
+    required: false
+    default: 'CODENOTIFY'
 runs:
   using: 'docker'
   image: 'Dockerfile'

diff --git a/main.go b/main.go
@@ -49,7 +49,7 @@ func testableMain(stdout io.Writer, args []string) error {
 		return fmt.Errorf("error scanning lines from diff: %s\n%s", err, string(diff))
 	}
 
-	notifs, err := notifications(&gitfs{cwd: opts.cwd, rev: opts.baseRef}, paths)
+	notifs, err := notifications(&gitfs{cwd: opts.cwd, rev: opts.baseRef}, paths, opts.filename)
 	if err != nil {
 		return err
 	}
@@ -87,6 +87,7 @@ func cliOptions(stdout io.Writer, args []string) (*options, error) {
 	flags.StringVar(&opts.baseRef, "baseRef", "", "The base ref to use when computing the file diff.")
 	flags.StringVar(&opts.headRef, "headRef", "HEAD", "The head ref to use when computing the file diff.")
 	flags.StringVar(&opts.format, "format", "text", "The format of the output: text or markdown")
+	flags.StringVar(&opts.filename, "filename", "CODENOTIFY", "The filename in which file subscribers are defined")
 	v := *flags.Bool("verbose", false, "Verbose messages printed to stderr")
 
 	if v {
@@ -154,12 +155,18 @@ func githubActionOptions() (*options, error) {
 		return nil, err
 	}
 
+	filename := os.Getenv("INPUT_FILENAME")
+	if filename == "" {
+		return nil, fmt.Errorf("env var INPUT_FILENAME not set")
+	}
+
 	o := &options{
-		cwd:     cwd,
-		format:  "markdown",
-		baseRef: event.PullRequest.Base.Sha,
-		headRef: event.PullRequest.Head.Sha,
-		author:  "@" + event.PullRequest.User.Login,
+		cwd:      cwd,
+		format:   "markdown",
+		filename: filename,
+		baseRef:  event.PullRequest.Base.Sha,
+		headRef:  event.PullRequest.Head.Sha,
+		author:   "@" + event.PullRequest.User.Login,
 	}
 	o.print = commentOnGitHubPullRequest(o, event.PullRequest.NodeID)
 	return o, nil
@@ -363,12 +370,13 @@ func graphql(query string, variables map[string]interface{}, responseData interf
 }
 
 type options struct {
-	cwd     string
-	baseRef string
-	headRef string
-	format  string
-	author  string
-	print   func(notifs map[string][]string) error
+	cwd      string
+	baseRef  string
+	headRef  string
+	format   string
+	filename string
+	author   string
+	print    func(notifs map[string][]string) error
 }
 
 const markdownCommentTitle = "<!-- codenotify report -->\n"
@@ -394,7 +402,7 @@ func (o *options) writeNotifications(w io.Writer, notifs map[string][]string) er
 		return nil
 	case "markdown":
 		fmt.Fprint(w, markdownCommentTitle)
-		fmt.Fprintf(w, "Notifying subscribers in [CODENOTIFY](https://github.com/sourcegraph/codenotify) files for diff %s...%s.\n\n", o.baseRef, o.headRef)
+		fmt.Fprintf(w, "[Codenotify](https://github.com/sourcegraph/codenotify): Notifying subscribers in %s files for diff %s...%s.\n\n", o.filename, o.baseRef, o.headRef)
 		if len(notifs) == 0 {
 			fmt.Fprintln(w, "No notifications.")
 		} else {
@@ -420,10 +428,10 @@ func readLines(b []byte) ([]string, error) {
 	return lines, scanner.Err()
 }
 
-func notifications(fs FS, paths []string) (map[string][]string, error) {
+func notifications(fs FS, paths []string, notifyFilename string) (map[string][]string, error) {
 	notifications := map[string][]string{}
 	for _, path := range paths {
-		subs, err := subscribers(fs, path)
+		subs, err := subscribers(fs, path, notifyFilename)
 		if err != nil {
 			return nil, err
 		}
@@ -436,13 +444,14 @@ func notifications(fs FS, paths []string) (map[string][]string, error) {
 	return notifications, nil
 }
 
-func subscribers(fs FS, path string) ([]string, error) {
+func subscribers(fs FS, path string, notifyFilename string) ([]string, error) {
+	fmt.Fprintf(verbose, "analyzing subscribers in %s files\n", notifyFilename)
 	subscribers := []string{}
 
 	parts := strings.Split(path, string(os.PathSeparator))
 	for i := range parts {
 		base := filepath.Join(parts[:i]...)
-		rulefilepath := filepath.Join(base, "CODENOTIFY")
+		rulefilepath := filepath.Join(base, notifyFilename)
 
 		rulefile, err := fs.Open(rulefilepath)
 		if err != nil {

diff --git a/main_test.go b/main_test.go
@@ -139,24 +139,26 @@ func TestWriteNotifications(t *testing.T) {
 		{
 			name: "empty markdown",
 			opts: options{
-				format:  "markdown",
-				baseRef: "a",
-				headRef: "b",
+				filename: "CODENOTIFY",
+				format:   "markdown",
+				baseRef:  "a",
+				headRef:  "b",
 			},
 			notifs: nil,
 			output: []string{
 				"<!-- codenotify report -->",
-				"Notifying subscribers in [CODENOTIFY](https://github.com/sourcegraph/codenotify) files for diff a...b.",
+				"[Codenotify](https://github.com/sourcegraph/codenotify): Notifying subscribers in CODENOTIFY files for diff a...b.",
 				"",
 				"No notifications.",
 			},
 		},
 		{
 			name: "empty text",
 			opts: options{
-				format:  "text",
-				baseRef: "a",
-				headRef: "b",
+				filename: "CODENOTIFY",
+				format:   "text",
+				baseRef:  "a",
+				headRef:  "b",
 			},
 			notifs: nil,
 			output: []string{
@@ -167,17 +169,18 @@ func TestWriteNotifications(t *testing.T) {
 		{
 			name: "markdown",
 			opts: options{
-				format:  "markdown",
-				baseRef: "a",
-				headRef: "b",
+				filename: "CODENOTIFY",
+				format:   "markdown",
+				baseRef:  "a",
+				headRef:  "b",
 			},
 			notifs: map[string][]string{
 				"@go": {"file.go", "dir/file.go"},
 				"@js": {"file.js", "dir/file.js"},
 			},
 			output: []string{
 				"<!-- codenotify report -->",
-				"Notifying subscribers in [CODENOTIFY](https://github.com/sourcegraph/codenotify) files for diff a...b.",
+				"[Codenotify](https://github.com/sourcegraph/codenotify): Notifying subscribers in CODENOTIFY files for diff a...b.",
 				"",
 				"| Notify | File(s) |",
 				"|-|-|",
@@ -188,9 +191,10 @@ func TestWriteNotifications(t *testing.T) {
 		{
 			name: "text",
 			opts: options{
-				format:  "text",
-				baseRef: "a",
-				headRef: "b",
+				filename: "CODENOTIFY",
+				format:   "text",
+				baseRef:  "a",
+				headRef:  "b",
 			},
 			notifs: map[string][]string{
 				"@go": {"file.go", "dir/file.go"},
@@ -244,11 +248,13 @@ func joinLines(lines []string) string {
 func TestNotifications(t *testing.T) {
 	tests := []struct {
 		name          string
+		filename      string
 		fs            memfs
 		notifications map[string][]string
 	}{
 		{
-			name: "no notifications",
+			name:     "no notifications",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "nomatch.md @notify\n",
 				"file.md":         "",
@@ -258,7 +264,8 @@ func TestNotifications(t *testing.T) {
 			notifications: nil,
 		},
 		{
-			name: "file.md",
+			name:     "file.md",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "file.md @notify\n",
 				"file.md":         "",
@@ -270,7 +277,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "no leading slash",
+			name:     "no leading slash",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "/file.md @notify\n",
 				"file.md":         "",
@@ -280,7 +288,8 @@ func TestNotifications(t *testing.T) {
 			notifications: nil,
 		},
 		{
-			name: "whitespace",
+			name:     "whitespace",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "\n\nfile.md @notify\n\n",
 				"file.md":         "",
@@ -292,7 +301,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "comments",
+			name:     "comments",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY": "#comment\n" +
 					"file.md @notify\n",
@@ -305,7 +315,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "*",
+			name:     "*",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "* @notify\n",
 				"file.md":         "",
@@ -317,7 +328,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "dir/*",
+			name:     "dir/*",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "dir/* @notify\n",
 				"file.md":         "",
@@ -329,7 +341,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "**",
+			name:     "**",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "** @notify\n",
 				"file.md":         "",
@@ -341,7 +354,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "**/*", // same as **
+			name:     "**/*", // same as **
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "**/* @notify\n",
 				"file.md":         "",
@@ -353,7 +367,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "**/file.md",
+			name:     "**/file.md",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "**/file.md @notify\n",
 				"file.md":         "",
@@ -365,7 +380,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "dir/**",
+			name:     "dir/**",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "dir/** @notify\n",
 				"file.md":         "",
@@ -377,7 +393,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "dir/", // same as "dir/**"
+			name:     "dir/", // same as "dir/**"
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "dir/ @notify\n",
 				"file.md":         "",
@@ -389,7 +406,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "dir/**/file.md",
+			name:     "dir/**/file.md",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY":      "dir/**/file.md @notify\n",
 				"file.md":         "",
@@ -402,7 +420,8 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "multiple subscribers",
+			name:     "multiple subscribers",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY": "* @alice @bob\n",
 				"file.md":    "",
@@ -413,15 +432,17 @@ func TestNotifications(t *testing.T) {
 			},
 		},
 		{
-			name: "..",
+			name:     "..",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"dir/CODENOTIFY": "../* @alice @bob\n",
 				"file.md":        "",
 			},
 			notifications: nil,
 		},
 		{
-			name: "multiple CODENOTIFY",
+			name:     "multiple CODENOTIFY",
+			filename: "CODENOTIFY",
 			fs: memfs{
 				"CODENOTIFY": "\n" +
 					"* @rootany\n" +
@@ -547,11 +568,37 @@ func TestNotifications(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:     "no notifications for OWNERS",
+			filename: "OWNERS",
+			fs: memfs{
+				"CODENOTIFY":      "file.md @notify\n",
+				"OWNERS":          "nomatch.md @notify\n",
+				"file.md":         "",
+				"dir/file.md":     "",
+				"dir/dir/file.md": "",
+			},
+			notifications: nil,
+		},
+		{
+			name:     "file.md in OWNERS",
+			filename: "OWNERS",
+			fs: memfs{
+				"CODENOTIFY":      "nomatch.md @notify\n",
+				"OWNERS":          "file.md @notify\n",
+				"file.md":         "",
+				"dir/file.md":     "",
+				"dir/dir/file.md": "",
+			},
+			notifications: map[string][]string{
+				"@notify": {"file.md"},
+			},
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			notifs, err := notifications(test.fs, test.fs.paths())
+			notifs, err := notifications(test.fs, test.fs.paths(), test.filename)
 			if err != nil {
 				t.Errorf("expected nil error; got %s", err)
 			}