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

filesystem add file meta methods #92

Merged
merged 3 commits into from
Apr 29, 2023

Conversation

dhanusaputra
Copy link
Contributor

close goravel/goravel#94

  • size is already implemented
  • add new dependency mimetype package

@codecov
Copy link

codecov bot commented Apr 12, 2023

Codecov Report

Patch coverage: 16.21% and project coverage change: -0.27 ⚠️

Comparison is base (eb2ef23) 51.52% compared to head (4b5d463) 51.25%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
- Coverage   51.52%   51.25%   -0.27%     
==========================================
  Files         103      103              
  Lines        7028     7102      +74     
==========================================
+ Hits         3621     3640      +19     
- Misses       3126     3178      +52     
- Partials      281      284       +3     
Impacted Files Coverage Δ
filesystem/cos.go 0.00% <0.00%> (ø)
filesystem/minio.go 0.00% <0.00%> (ø)
filesystem/oss.go 0.00% <0.00%> (ø)
filesystem/s3.go 0.00% <0.00%> (ø)
filesystem/local.go 11.24% <100.00%> (+11.24%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Thanks, a perfect PR, just a question, please optimize the unit tests and rebase master.

func (r *Local) MakeDirectory(directory string) error {
return os.MkdirAll(path.Dir(r.fullPath(directory)+"/"), os.ModePerm)
}

func (r *Local) MimeType(file string) (string, error) {
mtype, err := mimetype.DetectFile(r.fullPath(file))
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: We have a third package to get minetype now: github.com/h2non/filetype, can we use it?

@dhanusaputra dhanusaputra force-pushed the filesystem-add-filemeta branch from 250fee6 to 8a3a68f Compare April 13, 2023 14:54
@dhanusaputra
Copy link
Contributor Author

seems adding unit test with external method like minio, s3.HeadObject, etc. is not easy
so only added local
please info if there's easy way, thanks

@dhanusaputra dhanusaputra requested a review from hwbrzzl April 13, 2023 15:01
@hwbrzzl
Copy link
Contributor

hwbrzzl commented Apr 14, 2023

seems adding unit test with external method like minio, s3.HeadObject, etc. is not easy so only added local please info if there's easy way, thanks

Oh, yes, let me do it, I'll merge this PR when I start, maybe one week. Thanks for your hard work on this PR!

Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

LGTM

@hwbrzzl hwbrzzl merged commit d68baf0 into goravel:master Apr 29, 2023
@hwbrzzl
Copy link
Contributor

hwbrzzl commented Dec 18, 2024

Hey @dhanusaputra We created a private channel in Discord for contributors. You can click the link and DM me (@bowen) to join it if you are interested.

Here you can:

  1. Send PR link or review others' PRs;
  2. Discuss new features before creating an issue;
  3. Vote for something;
  4. etc.

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.

✨ [Feature] filesystem add file meta methods
2 participants