Merge pull request #8 from OffchainLabs/improve-ux

Improve ux

changelog/changelog.go

@@ -15,7 +15,7 @@ import (
 )
 
 var versionRE = regexp.MustCompile(`^#+ \[(v\d+\.\d+\.\d+)\]`)
-var sectionRE = regexp.MustCompile(`^### (\w+)\s?$`)
+var sectionRE = regexp.MustCompile(`^#{1,6} (\w+)\s?$`)
 
 const preamble = `# Changelog
 
@@ -242,6 +242,10 @@ func ParseFragment(lines []string, pr string) map[string][]string {
 			current = section
 			continue
 		}
+		// We don't have a way to categorize a bullet found outside a known section.
+		if current == "" {
+			continue
+		}
 		bullet := parseBullet(line, pr)
 		if bullet == "" {
 			continue
@@ -266,7 +270,7 @@ func ValidSections(sections map[string][]string) error {
 			if k == sectionIgnored {
 				continue
 			}
-			return fmt.Errorf("invalid section name: %s", k)
+			return fmt.Errorf("invalid section name in fragment: %s", k)
 		}
 	}
 	return nil

changelog/changelog_test.go

@@ -16,3 +16,24 @@ func TestParseBulletOverride(t *testing.T) {
 		t.Error("parseBullet did not recognize the override")
 	}
 }
+
+func TestSectionRe(t *testing.T) {
+	cases := []struct {
+		value string
+		valid bool
+	}{
+		{value: "####### Fixes", valid: false}, // there's no H7
+		{value: " Fixes", valid: false},
+		{value: "# Fixes", valid: true},
+		{value: "## Fixes", valid: true},
+		{value: "### Fixes", valid: true},
+		{value: "#### Fixes", valid: true},
+		{value: "##### Fixes", valid: true},
+		{value: "###### Fixes", valid: true},
+	}
+	for _, c := range cases {
+		if valid := sectionRE.MatchString(c.value); valid != c.valid {
+			t.Errorf("sectionRE failed to match %v", c)
+		}
+	}
+}

cmd/check/main.go

@@ -16,22 +16,40 @@ type githubConf struct {
 	FragmentListingEnv string // name of env var containing a list of file fragments
 }
 
-func parseArgs(args []string) (*changelog.Config, *githubConf, error) {
+func parseArgs(args []string) (c *changelog.Config, envConf *githubConf, err error) {
 	flags := flag.NewFlagSet("check", flag.ContinueOnError)
-	c := &changelog.Config{RepoConfig: changelog.RepoConfig{Owner: "prysmaticlabs", Repo: "prysm"}, ReleaseTime: time.Now()}
+	flags.Usage = func() {
+		fmt.Fprintf(flag.CommandLine.Output(), "Usage of %s:\n", os.Args[0])
+		flags.PrintDefaults()
+		fmt.Fprint(flag.CommandLine.Output(), "\n")
+	}
+	defer func() {
+		if err != nil {
+			flags.Usage()
+		}
+	}()
+
+	c = &changelog.Config{RepoConfig: changelog.RepoConfig{Owner: "prysmaticlabs", Repo: "prysm"}, ReleaseTime: time.Now()}
 	flags.StringVar(&c.RepoPath, "repo", "", "Path to the git repository")
 	flags.StringVar(&c.ChangesDir, "changelog-dir", "changelog", "Path to the directory containing changelog fragments for each commit")
 	flags.StringVar(&c.RepoConfig.MainRev, "main-rev", "origin/develop", "Main branch tip revision")
 	flags.StringVar(&c.Branch, "branch", "HEAD", "branch tip revision")
 	envCfg := &githubConf{}
 	flags.StringVar(&envCfg.FragmentListingEnv, "fragment-env", "", "Name of the environment variable containing a list of changelog fragments")
 	flags.Parse(args)
+	if c.RepoPath == "" {
+		wd, err := os.Getwd()
+		if err != nil {
+			return nil, envCfg, fmt.Errorf("repo flag not set and can't get working directory from syscall, %w", err)
+		}
+		c.RepoPath = wd
+	}
 	if envCfg.FragmentListingEnv != "" {
 		return nil, envCfg, nil
 	}
 
 	if c.RepoPath == "" {
-		return c, nil, fmt.Errorf("repo is required")
+		return c, nil, fmt.Errorf("-repo flag is required")
 	}
 	return c, nil, nil
 }
@@ -75,6 +93,9 @@ func checkFragments(envCfg *githubConf) error {
 		}
 		lines := strings.Split(string(b), "\n")
 		parsed := changelog.ParseFragment(lines, "")
+		if len(parsed) == 0 {
+			return fmt.Errorf("please check formatting of fragment file at %s, could not find any bullet points within valid sections", p)
+		}
 		for k, v := range parsed {
 			if len(v) == 0 {
 				delete(parsed, k)

cmd/release/main.go

@@ -11,9 +11,19 @@ import (
 	"github.com/OffchainLabs/unclog/changelog"
 )
 
-func parseArgs(args []string) (*changelog.Config, error) {
+func parseArgs(args []string) (c *changelog.Config, err error) {
 	flags := flag.NewFlagSet("release", flag.ContinueOnError)
-	c := &changelog.Config{RepoConfig: changelog.RepoConfig{Owner: "prysmaticlabs", Repo: "prysm"}, ReleaseTime: time.Now()}
+	flags.Usage = func() {
+		fmt.Fprintf(flag.CommandLine.Output(), "Usage of %s:\n", os.Args[0])
+		flags.PrintDefaults()
+		fmt.Fprint(flag.CommandLine.Output(), "\n")
+	}
+	defer func() {
+		if err != nil {
+			flags.Usage()
+		}
+	}()
+	c = &changelog.Config{RepoConfig: changelog.RepoConfig{Owner: "prysmaticlabs", Repo: "prysm"}, ReleaseTime: time.Now()}
 	flags.StringVar(&c.RepoPath, "repo", "", "Path to the git repository")
 	flags.StringVar(&c.ChangesDir, "changelog-dir", "changelog", "Path to the directory containing changelog fragments for each commit")
 	flags.StringVar(&c.Tag, "tag", "", "New release tag (must already exist in repo)")

log/kasey_improve-ux.md

@@ -0,0 +1,8 @@
+### Added
+- Output usage when a required param is missing.
+
+### Fixed
+- Bug parsing bullet points outside of valid section, causing a confusing error message.
+
+### Changed
+- Relax heading format so that H1-H6 all allowed.