Skip to content
This repository has been archived by the owner on Mar 22, 2022. It is now read-only.

lint: Check for README presence (and supporting commits) #98

Merged
merged 3 commits into from
Feb 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions cmd/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ func lintTrack(path string) bool {
check: missingMetadata,
msg: "An implementation for '%v' was found, but config.json does not reference this exercise.",
},
{
check: missingReadme,
msg: "The implementation for '%v' is missing a README.",
},
{
check: missingSolution,
msg: "The implementation for '%v' is missing an example solution.",
Expand Down Expand Up @@ -201,6 +205,25 @@ func missingSolution(t track.Track) []string {
return slugs
}

func missingReadme(t track.Track) []string {
readmes := map[string]bool{}
for _, exercise := range t.Exercises {
readmes[exercise.Slug] = exercise.HasReadme()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might check and only add if non-exist to make it faster for the second iteration since tracks generally will have readmes. And also for foregone, we can delete added keys. In the end, we can iterate smaller map and make a slice without checking.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion. There's a couple of lint checks that could benefit from this refactor.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really was thinking that missingReadme, missingSolution, missingTestSuite should really all be refactored into one function that parameterises over HasReadme, IsValid (what a name...), HasTestSuite respectively. They're all the same except for that one line, I think. And then this improvement can be applied to them all.

}
// Don't complain about missing readmes in foregone exercises.
for _, slug := range t.Config.ForegoneSlugs {
readmes[slug] = true
}

slugs := []string{}
for slug, ok := range readmes {
if !ok {
slugs = append(slugs, slug)
}
}
return slugs
}

func missingTestSuite(t track.Track) []string {
tests := map[string]bool{}
for _, exercise := range t.Exercises {
Expand Down
26 changes: 26 additions & 0 deletions cmd/lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ func TestLintTrack(t *testing.T) {
path: "../fixtures/broken-maintainers",
expected: true,
},
{
desc: "should fail when given a track missing READMEs.",
path: "../fixtures/missing-readme",
expected: true,
},
{
desc: "should not fail when given a track with all of its bits in place.",
path: "../fixtures/lint/valid-track",
Expand Down Expand Up @@ -110,6 +115,27 @@ func TestMissingMetadata(t *testing.T) {
assert.Equal(t, "cherry", slugs[1])
}

func TestMissingReadme(t *testing.T) {
track := track.Track{
Exercises: []track.Exercise{
{Slug: "apple"},
{Slug: "banana", ReadmePath: "README.md"},
{Slug: "cherry"},
},
}

slugs := missingReadme(track)

if len(slugs) != 2 {
t.Fatalf("Expected missing READMEs in 2 exercises, missing in %d", len(slugs))
}

sort.Strings(slugs)

assert.Equal(t, "apple", slugs[0])
assert.Equal(t, "cherry", slugs[1])
}

func TestMissingSolution(t *testing.T) {
track := track.Track{
Exercises: []track.Exercise{
Expand Down
Empty file.
14 changes: 14 additions & 0 deletions fixtures/missing-readme/config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"slug": "missing-readme",
"language": "Missing Readme",
"repository": "https://github.com/exercism/missing-readme",
"active": true,
"exercises": [
{
"uuid": "missingmissing",
"slug": "missing-readme",
"topics": [],
"difficulty": 1
}
]
}
Empty file.
Empty file.
Empty file.
Empty file.
11 changes: 11 additions & 0 deletions track/exercise.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
// Exercise is an implementation of an Exercism exercise.
type Exercise struct {
Slug string
ReadmePath string
SolutionPath string
TestSuitePath string
}
Expand All @@ -31,6 +32,11 @@ func NewExercise(root string, pg PatternGroup) (Exercise, error) {
return ex, err
}

err = setPath(root, "README\\.md", &ex.ReadmePath)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not seeing a need to let this be configurable, so it is not in PatternGroup

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me 👍

if err != nil {
return ex, err
}

return ex, err
}

Expand Down Expand Up @@ -62,6 +68,11 @@ func setPath(root, pattern string, field *string) error {
return filepath.Walk(root, walkFn)
}

// HasReadme checks that an exercise has a README.
func (ex Exercise) HasReadme() bool {
return ex.ReadmePath != ""
}

// HasTestSuite checks that an exercise has a test suite.
func (ex Exercise) HasTestSuite() bool {
return ex.TestSuitePath != ""
Expand Down