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.

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

Dmgutils #4

Merged
merged 4 commits into from
Feb 24, 2018
Merged

Dmgutils #4

merged 4 commits into from
Feb 24, 2018

Conversation

clburlison
Copy link
Contributor

This works and is quite helpful for a project I'm working on right now.

I'd like some feedback on the following areas:

  1. dmgutils okay for name?
  2. Since this requires "github.com/groob/plist" should I add a Gopkg.toml file to the root directory?
  3. One thing that I've found helpful is a function to check the mountpath and return a file listing. Would you suggest including the snippet below or is that getting to custom for a library?
    // checkExt - Check a directory for files with a specific extension;
    // return a listing of files found.
    func checkExt(ext string, path string) []string {
        var files []string
        filepath.Walk(path, func(path string, f os.FileInfo, _ error) error {
            if !f.IsDir() {
                r, err := regexp.MatchString(ext, f.Name())
                if err == nil && r {
                    files = append(files, f.Name())
                }
            }
            return nil
        })
        return files
    }
  4. Testing? Besides including a small dmg file in the repo, doing a mount, then unmount I'm not sure how else I could add a valid test case.

@groob
Copy link
Owner

groob commented Feb 24, 2018

I should probably stress out in the README that this whole repo is for testing ideas and you'd be mad to import this repo into another project. Copy/Paste the code into a real project is better.

having said that, thanks for contributing something useful! I think adding a Gopkg would be great, but can be done in a followup PR. I'm going to merge this now!

@groob groob merged commit cd94886 into groob:master Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants