Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] Refactor as importable module #124

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

[WIP] Refactor as importable module #124

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
051a9c9
Break logic out from main into Run()
CalebAlbers Aug 10, 2022
34a0cf1
Move legacy extensionFlag parsing to main
CalebAlbers Aug 10, 2022
bf55c39
Move error printing from Run() to main()
CalebAlbers Aug 10, 2022
3f570ec
Switch to injecting logger
CalebAlbers Aug 10, 2022
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
86 changes: 64 additions & 22 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,35 +114,75 @@ func main() {
os.Exit(1)
}

// Get non-flag command-line args
patterns := flag.Args()

// convert -skip flags to -ignore equivalents
for _, s := range skipExtensionFlags {
ignorePatterns = append(ignorePatterns, fmt.Sprintf("**/*.%s", s))
}

// create logger to print updates to stdout
logger := log.Default()

// real main
err := Run(
ignorePatterns,
spdx,
*holder,
*license,
*licensef,
*year,
*verbose,
*checkonly,
patterns,
logger,
)

if err != nil {
log.Fatal(err)
}
}

// Run executes addLicense with supplied variables
func Run(
ignorePatterns stringSlice,
spdx spdxFlag,
holder string,
license string,
licensef string,
year string,
Comment on lines +150 to +154
Copy link
Author

Choose a reason for hiding this comment

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

Combining stuff like holder, year, and license into a licenseData struct and passing that in would probably feel more appropriate.

verbose bool,
checkonly bool,
Copy link
Author

Choose a reason for hiding this comment

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

checkonly gets a bit weird here, since this function currently calls os.Exit(1) if there are files that need changes, which is definitely not consumable. Perhaps an alternative would be to return a boolean that represents if changes are needed or were made. From there, a consumer of this function could infer that if they pass checkonly = true and get back a true, it means files need to be modified. If checkonly = false and they get back a true, it means files were modified.

I don't love it, but I do like it more than returning an error. Just thinking out loud, though 🙂

patterns []string,
logger *log.Logger,
) error {

// verify that all ignorePatterns are valid
for _, p := range ignorePatterns {
if !doublestar.ValidatePattern(p) {
log.Fatalf("-ignore pattern %q is not valid", p)
return fmt.Errorf("-ignore pattern %q is not valid", p)
}
}

// map legacy license values
if t, ok := legacyLicenseTypes[*license]; ok {
*license = t
if t, ok := legacyLicenseTypes[license]; ok {
license = t
}

data := licenseData{
Year: *year,
Holder: *holder,
SPDXID: *license,
Year: year,
Holder: holder,
SPDXID: license,
}

tpl, err := fetchTemplate(*license, *licensef, spdx)
tpl, err := fetchTemplate(license, licensef, spdx)
if err != nil {
log.Fatal(err)
return err
}
t, err := template.New("").Parse(tpl)
if err != nil {
log.Fatal(err)
return err
}

// process at most 1000 files in parallel
Expand All @@ -153,11 +193,11 @@ func main() {
for f := range ch {
f := f // https://golang.org/doc/faq#closures_and_goroutines
wg.Go(func() error {
if *checkonly {
if checkonly {
// Check if file extension is known
lic, err := licenseHeader(f.path, t, data)
if err != nil {
log.Printf("%s: %v", f.path, err)
logger.Printf("%s: %v", f.path, err)
return err
}
if lic == nil { // Unknown fileExtension
Expand All @@ -166,21 +206,21 @@ func main() {
// Check if file has a license
hasLicense, err := fileHasLicense(f.path)
if err != nil {
log.Printf("%s: %v", f.path, err)
logger.Printf("%s: %v", f.path, err)
return err
}
if !hasLicense {
fmt.Printf("%s\n", f.path)
logger.Printf("%s\n", f.path)
return errors.New("missing license header")
}
} else {
modified, err := addLicense(f.path, f.mode, t, data)
if err != nil {
log.Printf("%s: %v", f.path, err)
logger.Printf("%s: %v", f.path, err)
return err
}
if *verbose && modified {
log.Printf("%s modified", f.path)
if verbose && modified {
logger.Printf("%s modified", f.path)
}
}
return nil
Expand All @@ -193,31 +233,33 @@ func main() {
}
}()

for _, d := range flag.Args() {
if err := walk(ch, d); err != nil {
log.Fatal(err)
for _, d := range patterns {
if err := walk(ch, d, logger); err != nil {
return err
}
}
close(ch)
<-done

return nil
}

type file struct {
path string
mode os.FileMode
}

func walk(ch chan<- *file, start string) error {
func walk(ch chan<- *file, start string, logger *log.Logger) error {
return filepath.Walk(start, func(path string, fi os.FileInfo, err error) error {
if err != nil {
log.Printf("%s error: %v", path, err)
logger.Printf("%s error: %v", path, err)
return nil
}
if fi.IsDir() {
return nil
}
if fileMatches(path, ignorePatterns) {
log.Printf("skipping: %s", path)
logger.Printf("skipping: %s", path)
return nil
}
ch <- &file{path, fi.Mode()}
Expand Down