-
Notifications
You must be signed in to change notification settings - Fork 12
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
[RFC] Feature #26 export to ical #27
base: master
Are you sure you want to change the base?
Conversation
534913f
to
4582c34
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/Users/joao/Desktop/golog/golog.go:15: exported const ExportICal should have comment (or a comment on this block) or be unexported
/Users/joao/Desktop/golog/golog.go:18: exported var ExporterNames should have comment or be unexported
/Users/joao/Desktop/golog/ical_exporter.go:5: exported const PRODID should have comment or be unexported
/Users/joao/Desktop/golog/ical_exporter.go:7: exported type ICalTaskExporter should have comment or be unexported
/Users/joao/Desktop/golog/ical_exporter.go:83: exported method ICalTaskExporter.Export should have comment or be unexported
/Users/joao/Desktop/golog/ical_exporter.go:121: exported method ICalTaskExporter.GetFileExtension should have comment or be unexported
/Users/joao/Desktop/golog/tasks.go:4: exported const TaskStart should have comment (or a comment on this block) or be unexported
ca4f20f
to
91f3bd0
Compare
To export run: ```sh golog export ``` Or: ```sh golog export ical golog.ics ``` Closes mlimaloureiro#26
b98ada7
to
5b7a65b
Compare
Version 1.1.2 is being removed due to and error on the github.com/codegangsta/cli@v1.20.0 dependency: ```` ../../codegangsta/cli/context.go:170: undefined: flag.Getter ``` Seems codegangsta/cli is using go's flag.Getter that was only [added in go v1.2](golang/go@49b3301) Other versions were replaced by 1.11.4 because the project started using go modules
5b7a65b
to
01e92f3
Compare
c2ba6a8
to
28bc77b
Compare
Add SetTask and SetTasks methods to TaskRepositoryInterface Implement the new TaskRepositoryInterface methods Add csv export format to the CLI Add comments to public structs / methods
28bc77b
to
67603ed
Compare
90a15c3
to
5ba1867
Compare
5ba1867
to
67eb470
Compare
models/tasks/activity_test.go
Outdated
|
||
t.Run("not running", func(t *testing.T) { | ||
taskActivity := TaskActivity{StartDate: timeFromString("2019-01-01T10:00:01Z"), EndDate: timeFromString("2019-01-02T10:00:00Z")} | ||
assert.Equal(t, 24*time.Hour-1*time.Second, taskActivity.Duration()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add space between operators for readability:
24 * time.Hour - 1 * ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go fmt
removes the spaces there.
// TaskActivity represents a date interval of a task execution | ||
type TaskActivity struct { | ||
StartDate time.Time | ||
EndDate time.Time // EndDate.IsZero() indicates a running task, meaning the 0001-01-01 EndDate cannot be represented |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have in line comments, or comments in the previous line ? let's also add this to the standards if we don't have it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have to pick only one i would vote for comments in the previous line. Helps keeping line length low and provides a explanation / comment before the actual code
But do we really need to pick only one? Sometimes i think it makes sense inline, like in this occurrence where it seems to make more sense to have the comment explaining the meaning of EndDate.IsZero()
after EndDate
has been declared.
package tasks // import github.com/mlimaloureiro/golog/repositories/tasks | ||
|
||
// DeleterTaskRepositoryInterface is a TaskRepositoryInterface that can also delete tasks directly | ||
type DeleterTaskRepositoryInterface interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what was the reason to have a dedicated interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some repositories might have implementations for this 2 operations (for example a SQL repository). This interface would allow TaskService to call those methods directly instead of using GetTasks() -> SetTasks() to delete tasks. Something similar to StarterPauserTaskRepositoryInterface.
But currently no repositories implement it.
Should i remove it?
func getFileContent(t *testing.T, filePath string) string { | ||
stringBytes, err := ioutil.ReadFile(filePath) | ||
assert.NoError(t, err) | ||
return string(stringBytes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
always add a new line in the last return, let's also add it to our styleguide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean like this:
func getFileContent(t *testing.T, filePath string) string {
stringBytes, err := ioutil.ReadFile(filePath)
assert.NoError(t, err)
return string(stringBytes)
}
?
When there is only one line this should be ignored right?
func GetPi() float64 {
return 3.14
}
No description provided.