add `tea times` command #54

Merged
lunny merged 20 commits from noerw/tea:50-cmd-times into master 2 years ago
noerw commented 3 years ago
Collaborator

As proposed in #50

TODO

  • --from, --to flags as filters on TrackedTime.Created

  • vendor new dependency araddon/dateparse

    • help needed, don't know how. put it in the README maybe? run make vendor!
  • don't show internal IDs, but proper user facing names / IDs:

    For some reason the API returns internal IssueID & UserID, instead of IssueIndex & UserName.
    I feel like it shouldn't be the clients job to resolve each of these internal IDs, so we might need an API change here?

  • delete times / reset issue: missing functions in go-sdk, so I skipped that one

As proposed in #50 ### TODO - [x] `--from`, `--to` flags as filters on `TrackedTime.Created` - [x] vendor new dependency `araddon/dateparse` - ~~help needed, don't know how. put it in the README maybe?~~ run `make vendor`! - [x] don't show internal IDs, but proper user facing names / IDs: For some reason the API returns internal IssueID & UserID, instead of IssueIndex & UserName. I feel like it shouldn't be the clients job to resolve each of these internal IDs, so we might need an API change here? - [x] delete times / reset issue: missing functions in go-sdk, so I skipped that one
lunny added the
kind/feature
label 3 years ago
Collaborator

I would wait for https://github.com/go-gitea/gitea/issues/8833 and it's stable upstream api

I would wait for https://github.com/go-gitea/gitea/issues/8833 and it's stable upstream api
6543 added the
status/blocked
label 3 years ago
Collaborator
status update: https://github.com/go-gitea/gitea/pull/9200 WIP ...
Collaborator

status update: 9200 is ready, need reviews

status update: 9200 is ready, need reviews
Collaborator

side notes:

  • can we check the gitea version? and if it is les than 1.11.0 print out that the instance is to old?
  • filter options for --from --to wil get into 1.12.x so if version is less priont out that version for this options is to less PR #9373

-> maby add general function(s) for this? as it could be used in other tea routines

side notes: * [ ] can we check the gitea version? and if it is les than 1.11.0 print out that the instance is to old? * [ ] filter options for --from --to wil get into 1.12.x so if version is less priont out that version for this options is to less [PR #9373](https://github.com/go-gitea/gitea/pull/9373) -> maby add general function(s) for this? as it could be used in other tea routines
6543 added
status/wip
and removed
status/blocked
labels 3 years ago
Collaborator

@noerw now nothing is blocking this PR anymore

@noerw now nothing is blocking this PR anymore
Owner

Is this still wip?

Is this still wip?
Poster
Collaborator

Yes,
if we're to add the gitea version check as proposed by @6543 in order to enable this feature, we're still blocked by #78 (since ServerVersion() is broken in the old vendored version of the go-sdk)

Yes, if we're to add the gitea version check as proposed by @6543 in order to enable this feature, we're still blocked by #78 (since `ServerVersion()` is broken in the old vendored version of the go-sdk)
6543 added a new dependency 3 years ago
6543 added this to the v0.2.0 milestone 3 years ago
6543 removed a dependency 3 years ago
Collaborator

@noerw new go-sdk merged

@noerw new go-sdk merged
noerw changed title from WIP: add `tea times` command to add `tea times` command 3 years ago
Poster
Collaborator

@6543 @lunny This is ready now:
Updated to latest master, version check is implemented, from/to filters are implemented client-side.

@6543 @lunny This is ready now: Updated to latest master, version check is implemented, from/to filters are implemented client-side.
techknowlogick reviewed 3 years ago
Dismissed
cmd/config.go Outdated
Insecure bool `yaml:"insecure"`
}
// Checks the logins server against a version constraint such as ">= 1.11.0+dev"
Owner

Build is failing due to this comment, it should start with CheckServerVersionConstraint

Build is failing due to this comment, it should start with `CheckServerVersionConstraint`
Collaborator

@noerw would be nice if you wold make code refactor in seperate PR next time :)

  • easier to review
  • easier lookup with git blame
  • changelog generation is simpler too
@noerw would be nice if you wold make code refactor in seperate PR next time :) - easier to review - easier lookup with git blame - changelog generation is simpler too
6543 removed the
status/wip
label 3 years ago
6543 started working 3 years ago
6543 stopped working 3 years ago
10min 9s
6543 added spent time 3 years ago
5min
6543 approved these changes 3 years ago
Dismissed
6543 left a comment
Collaborator

plreace split PR next time!

plreace split PR next time!
noerw added spent time 3 years ago
5h
Poster
Collaborator

@6543 its there:

USAGE:
   tea times command [command options] [username | #issue]

But I agree this solution is messy (always need to quote '#54' to avoid a bash comment).
We could do a more explicit variant with --user and --issue flags.

@6543 its there: ``` USAGE: tea times command [command options] [username | #issue] ``` But I agree this solution is messy (always need to quote `'#54'` to avoid a bash comment). We could do a more explicit variant with `--user` and `--issue` flags.
Poster
Collaborator

Testing tea times '#54' on this issue reveals that I can only list my own times on this issue, 6543s do not show up.

Is that intended / desired behaviour?

Testing `tea times '#54'` on this issue reveals that I can only list my own times on this issue, 6543s do not show up. Is that intended / desired behaviour?
Collaborator

@noerw no leave it like that we have enouth --options

@noerw no leave it like that we have enouth --options
Collaborator

@noerw it is, since you are no repoAdmin

... ok UI show this ... maby we should export this too :/

@noerw it is, since you are no repoAdmin ... ok UI show this ... maby we should export this too :/
Collaborator

"This pull request has changes conflicting with the target branch."

@noerw I thin you have to rebase this PR ...

"This pull request has changes conflicting with the target branch." @noerw I thin you have to rebase this PR ...
6543 added a new dependency 3 years ago
Collaborator

@noerw can you update to the latest sdk version?

v0.11.0

it also support deleting issues:

  • Client.ResetIssueTime(owner, repo string, index int64)
  • Client.DeleteTime(owner, repo string, index, timeID int64)

added in gitea/go-sdk#210

@noerw can you update to the latest sdk version? **v0.11.0** it also support deleting issues: * `Client.ResetIssueTime(owner, repo string, index int64)` * `Client.DeleteTime(owner, repo string, index, timeID int64)` added in https://gitea.com/gitea/go-sdk/pulls/210
6543 requested changes 3 years ago
Dismissed
6543 left a comment
Collaborator

update sdk to v0.11.0

update sdk to v0.11.0
noerw added spent time 2 years ago
1h
noerw added spent time 2 years ago
10min
noerw added spent time 2 years ago
10min
noerw deleted spent time 2 years ago
- 6h 20min
noerw added spent time 2 years ago
6h 20min
Poster
Collaborator

Ok, finally found some time to finalize this:

  • rebased feature branch onto master
  • now use CheckServerVersionConstraint from SDK v0.11
  • add delete & reset subcommands

@6543

Ok, finally found some time to finalize this: - rebased feature branch onto `master` - now use CheckServerVersionConstraint from SDK v0.11 - add `delete` & `reset` subcommands @6543
Owner

CI failed but I think #98 will fix that.

CI failed but I think #98 will fix that.
6543 reviewed 2 years ago
Dismissed
cmd/labels.go Outdated
Name: "save, s",
Usage: "Save all the labels as a file",
&cli.StringFlag{
Name: "save, s",
6543 commented 2 years ago
Collaborator

can you remove the s there?

can you remove the `s` there?
noerw commented 2 years ago
Poster
Collaborator

done

done
6543 requested changes 2 years ago
Dismissed
6543 left a comment
Collaborator

one nit

one nit
6543 added spent time 2 years ago
10min
6543 approved these changes 2 years ago
Dismissed
Collaborator

can you merge master in? or update this branch?

can you merge master in? or update this branch?
lunny approved these changes 2 years ago
Dismissed
lunny closed this pull request 2 years ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as 897e4ce3c1.
Sign in to join this conversation.
Loading…
There is no content yet.