Skip to content

Cobra refactor#2

Merged
fantasticrabbit merged 59 commits into
mainfrom
cobra-refactor
Nov 10, 2021
Merged

Cobra refactor#2
fantasticrabbit merged 59 commits into
mainfrom
cobra-refactor

Conversation

@fantasticrabbit

Copy link
Copy Markdown

No description provided.

@Andrew-Kulpa Andrew-Kulpa left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I love how this is turning out! I have some initial feedback on the subcommand structure and I noticed there are a lot of cobra auto-generated artifacts in the code.

Comment thread .github/workflows/go_build.yml Outdated
Comment thread Makefile Outdated
Comment thread Makefile Outdated
Comment thread Makefile Outdated
Comment thread cmd/get.go
Comment thread cmd/gettask.go Outdated
Comment on lines +23 to +33
if fileFlag == false {
fmt.Println(string(internal.GetClickUpTask(taskID, token, clientID)))
return
} else {
filenm := "clickup_" + taskID + ".json"
data := internal.GetClickUpTask(taskID, token, clientID)
err := os.WriteFile(filenm, data, 0644)
if err != nil {
fmt.Println("Error writing task JSON")
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why not just print to STDOUT and let the user redirect that output if they want to?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I agree with Andrew.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

STDOUT is the primary workflow, this is just an option to generate a standard file.

Comment thread cmd/logout.go

// logoutCmd represents the logout command
var logoutCmd = &cobra.Command{
Use: "logout",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you go forward with a clickup auth login this could just become clickup auth logout.

Comment thread cmd/root.go Outdated
Comment thread cmd/version.go Outdated
Comment thread cmd/root.go Outdated
@fantasticrabbit

Copy link
Copy Markdown
Author

Alright, I think I'm down to just 2 outstanding items:

  1. I like the auth flow I have here, per the discussion above, open to further discussion though.
  2. I think the get subcommand for task works better as a flag. Here's how I see it going:
    clickup get -t 12345
    clickup get --task 12345 --field start_date
    clickup get --bulktasks --list=18916883 --date_created_gt==20210101
    

There is not a lot of real data to GET in things other than tasks. The other GET functions really just provide metadata about a list, or a teamspace, etc., not any actual data. The important piece for GET will ensuring a clear distinction between doing a single task request and a bulk task request based on some field criteria (Not proposing --bulktasks here, just using for illustration).

@fantasticrabbit

Copy link
Copy Markdown
Author

• Resolved bug with ~/.clickup/config.yaml file not creating properly.
• moved get task to a proper sub command, passing taskID as an argument, (and added optional filtering for the # character that the Clickup web app tends to add when using it's copy-button function)

Comment thread Makefile
Comment thread README.md
Comment thread cmd/task.go
Comment thread README.md
Comment thread cmd/task.go
Comment thread internal/auth.go
Comment thread internal/auth.go
Comment thread cmd/root.go
Comment thread cmd/task.go
Comment thread cmd/task.go
@fantasticrabbit fantasticrabbit merged commit 179e0e6 into main Nov 10, 2021
@Andrew-Kulpa Andrew-Kulpa deleted the cobra-refactor branch July 26, 2023 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants