Skip to content

Commit 6b0bf66

Browse files
committed
refactor: update docs, logging, and contribution guidelines based on feedback
1 parent e396866 commit 6b0bf66

File tree

6 files changed

+91
-88
lines changed

6 files changed

+91
-88
lines changed

CONTRIBUTING.md

Lines changed: 55 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,21 @@
11
## Contribution Guidelines
2-
3-
- Minor changes can be done directly by editing code on GitHub. GitHub automatically creates a temporary branch and
2+
* Minor changes can be done directly by editing code on GitHub. GitHub automatically creates a temporary branch and
43
files a PR. This is only suitable for really small changes like: spelling fixes, variable name changes or error string
54
change etc. For larger commits, following steps are recommended.
6-
- (Optional) If you want to discuss your implementation with the users of GoFr, use the GitHub discussions of this repo.
7-
- Configure your editor to use goimport and golangci-lint on file changes. Any code which is not formatted using these
5+
* (Optional) If you want to discuss your implementation with the users of GoFr, use the GitHub discussions of this repo.
6+
* Configure your editor to use goimport and golangci-lint on file changes. Any code which is not formatted using these
87
tools, will fail on the pipeline.
9-
- Contributors should begin working on an issue only after it has been assigned to them. To get an issue assigned, please comment on the GitHub thread
8+
* Contributors should begin working on an issue only after it has been assigned to them. To get an issue assigned, please comment on the GitHub thread
109
and request assignment from a maintainer. This helps avoid duplicate or conflicting pull requests from multiple contributors.
11-
- Issues labeled triage are not open for direct contributions. If you're interested in working on a triage issue, please reach out to the maintainers
12-
to discuss it before proceeding in the Github thread.
10+
* Issues labeled triage are not open for direct contributions. If you're interested in working on a triage issue, please reach out to the maintainers
11+
to discuss it before proceeding in the Github thread.
1312
<!-- spellchecker:off "favour" have to be ignored here -->
14-
- We follow **American English** conventions in this project (e.g., _"favor"_ instead of _"favour"_). Please keep this consistent across all code comments, documentation, etc.
13+
* We follow **American English** conventions in this project (e.g., *"favor"* instead of *"favour"*). Please keep this consistent across all code comments, documentation, etc.
1514
<!-- spellchecker:on -->
16-
- All code contributions should have associated tests and all new line additions should be covered in those testcases.
15+
* All code contributions should have associated tests and all new line additions should be covered in those testcases.
1716
No PR should ever decrease the overall code coverage.
18-
- Once your code changes are done along with the testcases, submit a PR to development branch. Please note that all PRs
17+
* Once your code changes are done along with the testcases, submit a PR to development branch. Please note that all PRs
1918
are merged from feature branches to development first.
20-
2119
* PR should be raised only when development is complete and the code is ready for review. This approach helps reduce the number of open pull requests and facilitates a more efficient review process for the team.
2220
* All PRs need to be reviewed by at least 2 GoFr developers. They might reach out to you for any clarification.
2321
* Thank you for your contribution. :)
@@ -30,28 +28,32 @@ Testing is a crucial aspect of software development, and adherence to these guid
3028

3129
1. **Test Types:**
3230

33-
- Write unit tests for every new function or method.
34-
- Include integration tests for any major feature added.
31+
- Write unit tests for every new function or method.
32+
- Include integration tests for any major feature added.
3533

36-
2. **Test Coverage:**
3734

38-
- No new code should decrease the existing code coverage for the packages and files.
39-
> The `code-climate` coverage check will not pass if there is any decrease in the test-coverage before and after any new PR is submitted.
35+
2. **Test Coverage:**
4036

41-
3. **Naming Conventions:**
37+
- No new code should decrease the existing code coverage for the packages and files.
38+
> The `code-climate` coverage check will not pass if there is any decrease in the test-coverage before and after any new PR is submitted.
4239
43-
- Prefix unit test functions with `Test`.
44-
- Use clear and descriptive names.
4540

41+
42+
3. **Naming Conventions:**
43+
44+
- Prefix unit test functions with `Test`.
45+
- Use clear and descriptive names.
4646
```go
4747
func TestFunctionName(t *testing.T) {
4848
// Test logic
4949
}
5050
```
5151

52+
53+
5254
4. **Table-Driven Tests:**
5355

54-
- Consider using table-driven tests for testing multiple scenarios.
56+
- Consider using table-driven tests for testing multiple scenarios.
5557

5658
> [!NOTE]
5759
> Some services will be required to pass the entire test suite. We recommend using docker for running those services.
@@ -104,42 +106,41 @@ docker run -it --rm -p 4443:4443 -e STORAGE_EMULATOR_HOST=0.0.0.0:4443 fsouza/fa
104106
> [!NOTE]
105107
> Please note that the recommended local port for the services are different from the actual ports. This is done to avoid conflict with the local installation on developer machines. This method also allows a developer to work on multiple projects which uses the same services but bound on different ports. One can choose to change the port for these services. Just remember to add the same in configs/.local.env, if you decide to do that.
106108
107-
### Coding Guidelines
108109

109-
- Use only what is given to you as part of function parameter or receiver. No globals. Inject all dependencies including
110+
### Coding Guidelines
111+
* Use only what is given to you as part of function parameter or receiver. No globals. Inject all dependencies including
110112
DB, Logger etc.
111-
- No magic. So, no init. In a large project, it becomes difficult to track which package is doing what at the
113+
* No magic. So, no init. In a large project, it becomes difficult to track which package is doing what at the
112114
initialization step.
113-
- Exported functions must have an associated godoc.
114-
- Sensitive data(username, password, keys) should not be pushed. Always use environment variables.
115-
- Take interfaces and return concrete types.
116-
- Lean interfaces - take 'exactly' what you need, not more. Onus of interface definition is on the package who is
117-
using it. so, it should be as lean as possible. This makes it easier to test.
118-
- Be careful of type assertions in this context. If you take an interface and type assert to a type - then it's
119-
similar to taking concrete type.
120-
- Uses of context:
121-
- We should use context as a first parameter.
122-
- Can not use string as a key for the context. Define your own type and provide context accessor method to avoid
123-
conflict.
124-
- External Library uses:
125-
- A little copying is better than a little dependency.
126-
- All external dependencies should go through the same careful consideration, we would have done to our own written
127-
code. We need to test the functionality we are going to use from an external library, as sometimes library
128-
implementation may change.
129-
- All dependencies must be abstracted as an interface. This will make it easier to switch libraries at later point
130-
of time.
131-
- Version tagging as per Semantic versioning (https://semver.org/)
115+
* Exported functions must have an associated godoc.
116+
* Sensitive data(username, password, keys) should not be pushed. Always use environment variables.
117+
* Take interfaces and return concrete types.
118+
- Lean interfaces - take 'exactly' what you need, not more. Onus of interface definition is on the package who is
119+
using it. so, it should be as lean as possible. This makes it easier to test.
120+
- Be careful of type assertions in this context. If you take an interface and type assert to a type - then it's
121+
similar to taking concrete type.
122+
* Uses of context:
123+
- We should use context as a first parameter.
124+
- Can not use string as a key for the context. Define your own type and provide context accessor method to avoid
125+
conflict.
126+
* External Library uses:
127+
- A little copying is better than a little dependency.
128+
- All external dependencies should go through the same careful consideration, we would have done to our own written
129+
code. We need to test the functionality we are going to use from an external library, as sometimes library
130+
implementation may change.
131+
- All dependencies must be abstracted as an interface. This will make it easier to switch libraries at later point
132+
of time.
133+
* Version tagging as per Semantic versioning (https://semver.org/)
132134

133135
### Documentation
134-
135-
- After adding or modifying existing code, update the documentation too - [development/docs](https://github.com/gofr-dev/gofr/tree/development/docs).
136-
- When you consider a new documentation page is needed, start by adding a new file and writing your new documentation. Then - add a reference to it in [navigation.js](https://gofr.dev/docs/navigation.js).
137-
- If needed, update or add proper code examples for your changes.
138-
- In case images are needed, add it to [docs/public](./docs/public) folder.
139-
- Make sure you don't break existing links and references.
140-
- Maintain Markdown standards, you can read more [here](https://www.markdownguide.org/basic-syntax/), this includes:
141-
- Headings (`#`, `##`, etc.) should be placed in order.
142-
- Use trailing white space or the <br> HTML tag at the end of the line.
143-
- Use "`" sign to add single line code and "```" to add multi-line code block.
144-
- Use relative references to images (in `public` folder as mentioned above.)
145-
- The [gofr.dev documentation](https://gofr.dev/docs) site is updated upon push to `/docs` path in the repo. Verify your changes are live after next GoFr version.
136+
* After adding or modifying existing code, update the documentation too - [development/docs](https://github.com/gofr-dev/gofr/tree/development/docs).
137+
* When you consider a new documentation page is needed, start by adding a new file and writing your new documentation. Then - add a reference to it in [navigation.js](https://gofr.dev/docs/navigation.js).
138+
* If needed, update or add proper code examples for your changes.
139+
* In case images are needed, add it to [docs/public](./docs/public) folder.
140+
* Make sure you don't break existing links and references.
141+
* Maintain Markdown standards, you can read more [here](https://www.markdownguide.org/basic-syntax/), this includes:
142+
- Headings (`#`, `##`, etc.) should be placed in order.
143+
- Use trailing white space or the <br> HTML tag at the end of the line.
144+
- Use "`" sign to add single line code and "```" to add multi-line code block.
145+
- Use relative references to images (in `public` folder as mentioned above.)
146+
* The [gofr.dev documentation](https://gofr.dev/docs) site is updated upon push to `/docs` path in the repo. Verify your changes are live after next GoFr version.

docs/advanced-guide/handling-file/page.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ GoFr simplifies the complexity of working with different file stores by offering
66

77
By default, local file-store is initialized and user can access it from the context.
88

9-
GoFr also supports FTP/SFTP file-store. Developers can also connect and use their AWS S3 bucket or Google Cloud Storage (GCS) bucket as a file-store. The file-store can be initialized as follows:
9+
GoFr also supports FTP/SFTP file-store. Developers can also connect and use their cloud storage bucket as a file-store. Following cloud storage options are currently supported:
10+
11+
- **AWS S3**
12+
- **Google Cloud Storage (GCS)**
13+
14+
The file-store can be initialized as follows:
1015

1116
### FTP file-store
1217

@@ -307,7 +312,8 @@ err := ctx.File.Remove("my_dir")
307312

308313
The `RemoveAll` command deletes all subdirectories as well. If you delete the current working directory, such as "../currentDir", the working directory will be reset to its parent directory.
309314

310-
> Note: In S3 and GCS, RemoveAll only supports deleting directories and will return an error if a file path (as indicated by a file extension) is provided for S3. GCS handles both files and directories.
315+
> Note: In S3, RemoveAll only supports deleting directories and will return an error if a file path (as indicated by a file extension) is provided for S3.
316+
> GCS handles both files and directories.
311317
312318
```go
313319
err := ctx.File.RemoveAll("my_dir/my_text")

pkg/gofr/datasource/file/gcs/file.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ import (
99
"cloud.google.com/go/storage"
1010
)
1111

12-
// GCSFile represents a file in an GCS bucket.
13-
//
14-
//nolint:revive // gcs.GCSFile is repetitive. A better name could have been chosen, but it's too late as it's already exported.
15-
type GCSFile struct {
12+
type File struct {
1613
conn gcsClient
1714
writer *storage.Writer
1815
name string
@@ -39,14 +36,14 @@ const (
3936

4037
// ====== File interface methods ======
4138

42-
func (f *GCSFile) Read(p []byte) (int, error) {
39+
func (f *File) Read(p []byte) (int, error) {
4340
if f.body == nil {
4441
return 0, ErrNilGCSFileBody
4542
}
4643

4744
return f.body.Read(p)
4845
}
49-
func (f *GCSFile) Write(p []byte) (int, error) {
46+
func (f *File) Write(p []byte) (int, error) {
5047
bucketName := getBucketName(f.name)
5148

5249
var msg string
@@ -69,14 +66,13 @@ func (f *GCSFile) Write(p []byte) (int, error) {
6966
return n, err
7067
}
7168

72-
st = statusSuccess
73-
msg = "Write successful"
74-
f.logger.Logf(msg)
69+
st, msg = statusSuccess, "Write successful"
70+
f.logger.Debug(msg)
7571

7672
return n, nil
7773
}
7874

79-
func (f *GCSFile) Close() error {
75+
func (f *File) Close() error {
8076
bucketName := getBucketName(f.name)
8177

8278
var msg string
@@ -101,7 +97,7 @@ func (f *GCSFile) Close() error {
10197

10298
msg = msgWriterClosed
10399

104-
f.logger.Logf(msg)
100+
f.logger.Debug(msg)
105101

106102
return nil
107103
}
@@ -117,7 +113,7 @@ func (f *GCSFile) Close() error {
117113

118114
msg = msgReaderClosed
119115

120-
f.logger.Logf(msgReaderClosed)
116+
f.logger.Debug(msgReaderClosed)
121117

122118
return nil
123119
}
@@ -129,20 +125,20 @@ func (f *GCSFile) Close() error {
129125
return nil
130126
}
131127

132-
func (*GCSFile) Seek(_ int64, _ int) (int64, error) {
128+
func (*File) Seek(_ int64, _ int) (int64, error) {
133129
// Not supported: Seek requires reopening with range.
134130
return 0, ErrSeekNotSupported
135131
}
136132

137-
func (*GCSFile) ReadAt(_ []byte, _ int64) (int, error) {
133+
func (*File) ReadAt(_ []byte, _ int64) (int, error) {
138134
return 0, ErrReadAtNotSupported
139135
}
140136

141-
func (*GCSFile) WriteAt(_ []byte, _ int64) (int, error) {
137+
func (*File) WriteAt(_ []byte, _ int64) (int, error) {
142138
return 0, ErrWriteAtNotSupported
143139
}
144140

145-
func (f *GCSFile) sendOperationStats(fl *FileLog, startTime time.Time) {
141+
func (f *File) sendOperationStats(fl *FileLog, startTime time.Time) {
146142
duration := time.Since(startTime).Microseconds()
147143

148144
fl.Duration = duration

pkg/gofr/datasource/file/gcs/file_parse.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type jsonReader struct {
3838
token json.Token
3939
}
4040

41-
func (f *GCSFile) ReadAll() (file.RowReader, error) {
41+
func (f *File) ReadAll() (file.RowReader, error) {
4242
bucketName := getBucketName(f.name)
4343
location := path.Join(bucketName, f.name)
4444

@@ -55,7 +55,7 @@ func (f *GCSFile) ReadAll() (file.RowReader, error) {
5555
}
5656

5757
// createJSONReader creates a JSON reader for JSON files.
58-
func (f *GCSFile) createJSONReader(location string) (file.RowReader, error) {
58+
func (f *File) createJSONReader(location string) (file.RowReader, error) {
5959
status := statusErr
6060

6161
defer f.sendOperationStats(&FileLog{Operation: "JSON READER", Location: location, Status: &status}, time.Now())
@@ -92,7 +92,7 @@ func (f *GCSFile) createJSONReader(location string) (file.RowReader, error) {
9292
}
9393

9494
// createTextCSVReader creates a text reader for reading text files.
95-
func (f *GCSFile) createTextCSVReader(location string) (file.RowReader, error) {
95+
func (f *File) createTextCSVReader(location string) (file.RowReader, error) {
9696
status := statusErr
9797

9898
defer f.sendOperationStats(&FileLog{Operation: "TEXT/CSV READER", Location: location, Status: &status}, time.Now())
@@ -136,7 +136,7 @@ func (f *textReader) Scan(i any) error {
136136
return errStringNotPointer
137137
}
138138

139-
func (f *GCSFile) Name() string {
139+
func (f *File) Name() string {
140140
bucketName := getBucketName(f.name)
141141

142142
f.sendOperationStats(&FileLog{
@@ -147,7 +147,7 @@ func (f *GCSFile) Name() string {
147147
return f.name
148148
}
149149

150-
func (f *GCSFile) Size() int64 {
150+
func (f *File) Size() int64 {
151151
bucketName := getBucketName(f.name)
152152

153153
f.sendOperationStats(&FileLog{
@@ -158,7 +158,7 @@ func (f *GCSFile) Size() int64 {
158158
return f.size
159159
}
160160

161-
func (f *GCSFile) ModTime() time.Time {
161+
func (f *File) ModTime() time.Time {
162162
bucketName := getBucketName(f.name)
163163

164164
f.sendOperationStats(&FileLog{
@@ -169,7 +169,7 @@ func (f *GCSFile) ModTime() time.Time {
169169
return f.lastModified
170170
}
171171

172-
func (f *GCSFile) Mode() fs.FileMode {
172+
func (f *File) Mode() fs.FileMode {
173173
bucketName := getBucketName(f.name)
174174

175175
f.sendOperationStats(&FileLog{
@@ -184,7 +184,7 @@ func (f *GCSFile) Mode() fs.FileMode {
184184
return 0
185185
}
186186

187-
func (f *GCSFile) IsDir() bool {
187+
func (f *File) IsDir() bool {
188188
bucketName := getBucketName(f.name)
189189

190190
f.sendOperationStats(&FileLog{
@@ -195,7 +195,7 @@ func (f *GCSFile) IsDir() bool {
195195
return f.isDir || f.contentType == "application/x-directory"
196196
}
197197

198-
func (f *GCSFile) Sys() any {
198+
func (f *File) Sys() any {
199199
bucketName := getBucketName(f.name)
200200

201201
f.sendOperationStats(&FileLog{

pkg/gofr/datasource/file/gcs/fs.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ var (
1919
)
2020

2121
type FileSystem struct {
22-
GCSFile GCSFile
22+
GCSFile File
2323
conn gcsClient
2424
config *Config
2525
logger Logger
@@ -166,7 +166,7 @@ func (f *FileSystem) Create(name string) (file.File, error) {
166166

167167
f.logger.Logf("Write stream successfully opened for file %q", name)
168168

169-
return &GCSFile{
169+
return &File{
170170
conn: f.conn,
171171
writer: sw,
172172
name: name,
@@ -242,7 +242,7 @@ func (f *FileSystem) Open(name string) (file.File, error) {
242242

243243
msg = fmt.Sprintf("File with path %q retrieved successfully", name)
244244

245-
return &GCSFile{
245+
return &File{
246246
conn: f.conn,
247247
name: name,
248248
body: reader,

0 commit comments

Comments
 (0)