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

fix: reuse existing license if user didn't set any #104

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: reuse existing license if user didn't set any #104

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 11 additions & 1 deletion cmd/project.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,17 @@ func (p *Project) createLicenseFile() error {
data := map[string]interface{}{
"copyright": copyrightLine(),
}
licenseFile, err := os.Create(fmt.Sprintf("%s/LICENSE", p.AbsolutePath))

licensePath := p.AbsolutePath + "/LICENSE"
Copy link

Choose a reason for hiding this comment

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

This is nicer, but the rest of the code use the previous style so this create inconsistency.

It would be better to keep the current style for this change, and if changing the way paths are created is needed, do this in the entire file or project.


if p.Legal.Name == "None" {
// If user didn't set any license and an existing license file is found, use that.
if _, err := os.Stat(licensePath); err == nil {
return nil
}
}

licenseFile, err := os.Create(licensePath)
Copy link

Choose a reason for hiding this comment

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

Using exiting license makes sense, but this check ignores errors from os.Stat(), (other then fs.ErrNotExits). If the file exits and os.Stat failed, something is very wrong and failing is the best option.

But I think there is a better way - do we really want to overwrite existing license file if the user specify the license type? This does not feel like a careful way to modify user source tree.

So we can ensure that we never overwrite existing license, and handle the special case when the user did not specify the license, in which we will reuse the existing license.

Something like:

licenseFile, err := os.OpenFile(licensePath, O_CREATE|O_EXCL, 0755)
if err != nil {
    if errors.Is(err, fs.ErrExist) && p.Legal.Name == "None" {
        // User did not specify the license - use the existing one.
        return nil
    }
    return err
}

if err != nil {
return err
}
Expand Down