-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Added 'hours' & 'date' as optional flags to export #34
Added 'hours' & 'date' as optional flags to export #34
Conversation
Added both hours and date to both export types. Choose the 'string' datatype as that makes it easiest to fit in the way the project is structured. Both flags are optional and by default disabled. Build successfully with 'make VERSION=0.0.8' and tested both outputs with the optional flags. -> Output as desired.
I did not realise the 'GoFmt' formatting had that many changes. Now it's difficult to read the PR, I appologize. Please see the main differences here:
Please let me know if any questions arrise. |
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.
First of, thanks for the PR!
As you might have noticed, I didn't really care for a programming language dictating me what indentation to use. Hence there's a .editorconfig
in the repo root. Could you make your editor honor it and force push a reformatted version of your PR? It's really hard to go through, and I wouldn't want incremental changes in indentation across the code.
One point that I'm already seeing is the extension of Entry
. I think discarding the data using json:"-"
would be better, as we wouldn't want that information to somehow end up in the database.
Also, I saw that you added two range loops, one for each flag. I think it makes sense to consolidate them, just to save some time/computing power for very large exports. Wdyt?
I see, I mean in Go you have the benefit that it only has one offical formatter that can´t be configured so if you formatt it with it it's always the same, not like JS where you have 2000+ and each can confiigured differently. But I will integrate the .editorconfig to format it correctly as suggested, to make the PR more readable & conform to the project. Can you elaborate on which fields exactly to disregard? Cause anything we do '-' on, also won´t be exported in the end then as far as I tested it. Imo Go won´t suffer much time/performance loss in forloops unlike Python but I still can put it into one loop to make it nicer / more concise |
It's very difficult to enforce 'Go' to use spaces instead of tabs, as that's what the default formatter does. |
I will open a new PR, as this one I can´t rescue with the formatting. |
Additional posting this comment as well on this PR: Nope.. it does not let me push the code without it completly formatting. I'm sorry. I can't then do a PR. My apologies. If you still want to see the changes and commit them yourself you can view the PR by 'ignorign Whitespace' in the GIthub viewer. -> Define hours & date argument variables //...
var exportHours bool
var exportDate bool
//... -> Add Date & Hours to the entry struct
type Entry struct {
//...
Date string `json:"date,omitempty"`
Hours string`json:"hours,omitempty"`
//...
}
func (entry *Entry) SetDateFromBegin() {
entry.Date = entry.Begin.Format("02-03-2006") // DD-MM-YYYY
} -> Add hours & dates to the arguments
And lastly in I hope this helps at bit at least. |
Added both hours and date to both export types.
Choose the 'string' datatype as that makes it easiest to fit in the way the project is structured.
Both flags are optional and by default disabled.
Build successfully with 'make VERSION=0.0.8' and tested both outputs with the optional flags. -> Output as desired.
Let me know if any other changes are requiered or wanted, and I adapt as needed.
This should close #32