Skip to content

Commit

Permalink
feat(changelogutils): added activeSubdirectory field to "validation…
Browse files Browse the repository at this point in the history
….yaml" (#502)

* feat(changelogutils): added `activeSubdirectory` field to "validation.yaml"

This should enable users to have less sprawling changelog directorys as time progresses.  Consider a directory like...
```yaml
changelog:
  v1.0.0
  v1.0.1
  v1.0.2
  v1.1.0
  v1.1.1
  v1.1.2
  v1.1.3
```

By specifying a activeSubdirectory="v1.1", you could achieve a more readable structure of
```yaml
changelog:
  v1.0:
    v1.0.0
    v1.0.1
    v1.0.2
  v1.1
    v1.1.0
    v1.1.1
    v1.1.2
    v1.1.3
```

* adjusted no validation.yaml logic

* updated tests to account for previously hardcoded mocks
  • Loading branch information
gunnar-solo authored Sep 9, 2022
1 parent da8f13e commit 08d4573
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 83 deletions.
3 changes: 3 additions & 0 deletions changelog/v0.22.3/configurable-changelog-dir.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
changelog:
- type: NON_USER_FACING
description: added ability to validate changelogs in a subdirectory by specifying `activeSubdirectory` in "validation.yaml". This should enable users to have less sprawling changelog directorys as time progresses.
2 changes: 2 additions & 0 deletions changelogutils/changelog.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ type Changelog struct {
}

const (
// all functions in this file that use ChangelogDirectory are marked deprecated. The hope
// is we don't need to account for the "ActiveSubdirectory" setting which was added _after_ said deprecation
ChangelogDirectory = "changelog"
SummaryFile = "summary.md"
ClosingFile = "closing.md"
Expand Down
23 changes: 22 additions & 1 deletion changelogutils/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,25 @@ func NewChangelogReader(code vfsutils.MountedRepo) ChangelogReader {
return &changelogReader{code: code}
}

func (c *changelogReader) GetChangelogDirectory(ctx context.Context) string {
var settings ValidationSettings
bytes, err := c.code.GetFileContents(ctx, GetValidationSettingsPath())
if err != nil {
// unable to read validtion.yaml ~= "validation.yaml is not there"
return "changelog"
}

if err := yaml.Unmarshal(bytes, &settings); err != nil {
// suppressing error, because we _should_ always know the changelog dir
return "changelog"
}

if settings.ActiveSubdirectory != "" {
return "changelog/" + settings.ActiveSubdirectory
}
return "changelog"
}

func (c *changelogReader) GetChangelogForTag(ctx context.Context, tag string) (*Changelog, error) {
version, err := versionutils.ParseVersion(tag)
if err != nil {
Expand All @@ -60,7 +79,9 @@ func (c *changelogReader) GetChangelogForTag(ctx context.Context, tag string) (*
changelog := Changelog{
Version: version,
}
changelogPath := filepath.Join(ChangelogDirectory, tag)
dir := c.GetChangelogDirectory(ctx)

changelogPath := filepath.Join(dir, tag)
files, err := c.code.ListFiles(ctx, changelogPath)
if err != nil {
return nil, UnableToListFilesError(err, changelogPath)
Expand Down
18 changes: 18 additions & 0 deletions changelogutils/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ var _ = Describe("ReaderTest", func() {
mockCode.EXPECT().
ListFiles(ctx, changelogDir).
Return(nil, nestedErr)
mockCode.EXPECT().
GetFileContents(ctx, "changelog/validation.yaml").
Return([]byte(""), nil)

expected := changelogutils.UnableToListFilesError(nestedErr, changelogDir)
changelog, err := reader.GetChangelogForTag(ctx, tag)
Expand All @@ -106,6 +109,9 @@ var _ = Describe("ReaderTest", func() {
mockCode.EXPECT().
ListFiles(ctx, changelogDir).
Return(files, nil)
mockCode.EXPECT().
GetFileContents(ctx, "changelog/validation.yaml").
Return([]byte(""), nil)

expected := changelogutils.UnexpectedDirectoryError("foo", changelogDir)

Expand All @@ -123,6 +129,9 @@ var _ = Describe("ReaderTest", func() {
mockCode.EXPECT().
GetFileContents(ctx, path).
Return([]byte(contents), err)
mockCode.EXPECT().
GetFileContents(ctx, "changelog/validation.yaml").
Return([]byte(""), nil)
return path
}

Expand Down Expand Up @@ -220,6 +229,9 @@ var _ = Describe("ReaderTest", func() {
mockCode.EXPECT().
ListFiles(ctx, changelogDir).
Return(files, nil)
mockCode.EXPECT().
GetFileContents(ctx, "changelog/validation.yaml").
Return([]byte(""), nil)
mockCode.EXPECT().
GetFileContents(ctx, filepath.Join(changelogDir, "1.yaml")).
Return([]byte(validChangelog1), nil)
Expand Down Expand Up @@ -321,6 +333,9 @@ var _ = Describe("ReaderTest", func() {
mockCode.EXPECT().
ListFiles(ctx, rcDir).
Return(files, nil)
mockCode.EXPECT().
GetFileContents(ctx, "changelog/validation.yaml").
Return([]byte(""), nil)
mockCode.EXPECT().
GetFileContents(ctx, filepath.Join(rcDir, "1.yaml")).
Return([]byte(validChangelog3), nil)
Expand Down Expand Up @@ -353,6 +368,9 @@ var _ = Describe("ReaderTest", func() {
mockCode.EXPECT().
ListFiles(ctx, changelogDir).
Return(files, nil)
mockCode.EXPECT().
GetFileContents(ctx, "changelog/validation.yaml").
Return([]byte(""), nil)
mockCode.EXPECT().
GetFileContents(ctx, filepath.Join(changelogDir, "1.yaml")).
Return([]byte(validStableReleaseChangelog), nil)
Expand Down
31 changes: 26 additions & 5 deletions changelogutils/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ type ValidationSettings struct {

// If non-empty, then the validator will reject a changelog if the version's label is not contained in this slice
AllowedLabels []string `json:"allowedLabels"`

// defaults to "". When set, allows for a nested processing schema. ex: "v1.10" would mean only files in "changelog/v1.10" would be processed
ActiveSubdirectory string `json:"activeSubdirectory"`
}

type changelogValidator struct {
Expand All @@ -107,14 +110,15 @@ type changelogValidator struct {
}

func (c *changelogValidator) ShouldCheckChangelog(ctx context.Context) (bool, error) {
masterHasChangelog, err := c.client.DirectoryExists(ctx, MasterBranch, ChangelogDirectory)
dir := c.GetChangelogDirectory(ctx)
masterHasChangelog, err := c.client.DirectoryExists(ctx, MasterBranch, dir)
if err != nil {
return false, err
} else if masterHasChangelog {
return true, nil
}

branchHasChangelog, err := c.client.DirectoryExists(ctx, c.code.GetSha(), ChangelogDirectory)
branchHasChangelog, err := c.client.DirectoryExists(ctx, c.code.GetSha(), dir)
if err != nil {
return false, err
}
Expand All @@ -140,7 +144,7 @@ func (c *changelogValidator) ValidateChangelog(ctx context.Context) (*ChangelogF
}

// validate commit file for tag
if !strings.HasPrefix(commitFile.GetFilename(), fmt.Sprintf("%s/%s", ChangelogDirectory, proposedTag)) {
if !strings.HasPrefix(commitFile.GetFilename(), fmt.Sprintf("%s/%s", c.GetChangelogDirectory(ctx), proposedTag)) {
return nil, AddedChangelogInOldVersionError(proposedTag)
}

Expand All @@ -152,14 +156,16 @@ func (c *changelogValidator) validateProposedTag(ctx context.Context) (string, e
if err != nil {
return "", ListReleasesError(err)
}
children, err := c.code.ListFiles(ctx, ChangelogDirectory)

dir := c.GetChangelogDirectory(ctx)
children, err := c.code.ListFiles(ctx, dir)
if err != nil {
return "", err
}
proposedVersion := ""
for _, child := range children {
if !child.IsDir() {
if !IsKnownChangelogFile(filepath.Join(ChangelogDirectory, child.Name())) {
if !IsKnownChangelogFile(filepath.Join(dir, child.Name())) {
return "", UnexpectedFileInChangelogDirectoryError(child.Name())
} else {
continue
Expand Down Expand Up @@ -286,6 +292,7 @@ func GetChangelogFilesAdded(ctx context.Context, client githubutils.RepoClient,
}
var changelogFiles []github.CommitFile
for _, file := range commitComparison.Files {
// leaving ChangelogDirectory hardcoded to "changelog" is a non-issue here, since we are lazy prefix matching against "changelog/*"
if strings.HasPrefix(file.GetFilename(), fmt.Sprintf("%s/", ChangelogDirectory)) {
if !IsKnownChangelogFile(file.GetFilename()) && file.GetStatus() == githubutils.COMMIT_FILE_STATUS_ADDED {
changelogFiles = append(changelogFiles, *file)
Expand All @@ -296,6 +303,8 @@ func GetChangelogFilesAdded(ctx context.Context, client githubutils.RepoClient,
}

func GetValidationSettingsPath() string {
// leaving ChangelogDirectory hardcoded to "changelog" is a non-issue here, since even _if_ ActiveSubdirectory is set, we still only
// want to consider 1 (top-level) settings file
return fmt.Sprintf("%s/%s", ChangelogDirectory, ValidationSettingsFile)
}

Expand All @@ -307,6 +316,18 @@ func (c *changelogValidator) getValidationSettings(ctx context.Context) (*Valida
return GetValidationSettings(ctx, c.code, c.client)
}

func (c *changelogValidator) GetChangelogDirectory(ctx context.Context) string {
// as a potential optimization, we could make ValidationSettings a singleton to prevent continually reading from a remote GH
// during a single validation operation. I've elected to _not_ do so here, since I'm not totally sure if `changelogValidator`'s are
// used in a "1 and done" capacity. Calling this out "in case"
settings, _ := c.getValidationSettings(ctx) // suppressing error, because we _should_ always know the changelog dir

if settings != nil && settings.ActiveSubdirectory != "" {
return "changelog/" + settings.ActiveSubdirectory
}
return "changelog"
}

func GetValidationSettings(ctx context.Context, code vfsutils.MountedRepo, client githubutils.RepoClient) (*ValidationSettings, error) {
exists, err := client.FileExists(ctx, code.GetSha(), GetValidationSettingsPath())
if err != nil {
Expand Down
Loading

0 comments on commit 08d4573

Please sign in to comment.