Add interactive mode for `tea milestone create` #310

Merged
6543 merged 4 commits from YakoYakoYokuYoku/tea:interactive-ms into master 4 months ago

Fixes #245.

Fixes #245.
YakoYakoYokuYoku added 1 commit 4 months ago
52641083c9
Implement interactive milestone creation
noerw reviewed 4 months ago
modules/task/milestone_create.go Outdated
// title is required
if len(title) == 0 {
fmt.Printf("Title is required\n")
noerw commented 4 months ago
Poster

return fmt.Errorf("Title is required")

`return fmt.Errorf("Title is required")`
YakoYakoYokuYoku marked this conversation as resolved
noerw changed title from Add interactive mode for `tea issue create` to Add interactive mode for `tea milestone create` 4 months ago
noerw added the
kind/feature
label 4 months ago

Thanks 👍
Do you plan to ask for the deadline as well? That would be great as it prepares for #303

Thanks 👍 Do you plan to ask for the deadline as well? That would be great as it prepares for #303
noerw added this to the v0.7.0 milestone 4 months ago
YakoYakoYokuYoku added 1 commit 4 months ago
a0835d3a18
Return fmt.Errorf when title is empty

Do you plan to ask for the deadline as well? That would be great as it prepares for #303

Yeah, let's do that.

> Do you plan to ask for the deadline as well? That would be great as it prepares for #303 Yeah, let's do that.

@noerw May I ask you the date layout for the deadline (see time.Parse)?

@noerw May I ask you the date layout for the deadline (see [time.Parse](https://pkg.go.dev/time#Parse))?
Collaborator

by the way pleace "update branch" ...

by the way pleace "update branch" ...
6543 reviewed 4 months ago
cmd/milestones/create.go Outdated
func runMilestonesCreate(cmd *cli.Context) error {
ctx := context.InitCommand(cmd)
ctx.Ensure(context.CtxRequirement{RemoteRepo: true})
6543 commented 4 months ago
Poster
Collaborator

isn't ctx.Ensure(context.CtxRequirement{RemoteRepo: true}) still needed?

isn't `ctx.Ensure(context.CtxRequirement{RemoteRepo: true})` still needed?
6543 commented 4 months ago
Poster
Collaborator

cc @noerw

cc @noerw
noerw commented 4 months ago
Poster

It is.

It is.
YakoYakoYokuYoku marked this conversation as resolved
YakoYakoYokuYoku added 1 commit 4 months ago
a169550f62 Merge branch 'master' into interactive-ms

@YakoYakoYokuYoku Regarding time parsing: I'd do something similar to

var t time.Time
if deadline != "" {
	if t, err := time.ParseInLocation(time.RFC3339, deadline); err != nil {
		if t, err = time.ParseInLocation(time.UnixDate, deadline); err != nil {
			return err
		}
	}
}

edit: doe RFC3339 accept date only? If not, we should accept a date in RFC3339 structure, but with only the date..

@YakoYakoYokuYoku Regarding time parsing: I'd do something similar to ``` var t time.Time if deadline != "" { if t, err := time.ParseInLocation(time.RFC3339, deadline); err != nil { if t, err = time.ParseInLocation(time.UnixDate, deadline); err != nil { return err } } } ``` edit: doe RFC3339 accept date only? If not, we should accept a date in RFC3339 structure, but with only the date..
YakoYakoYokuYoku force-pushed interactive-ms from a169550f62 to 747fa1345f 4 months ago

doe RFC3339 accept date only? If not, we should accept a date in RFC3339 structure, but with only the date..

It seems that time.RFC3339 doesn't support an ISO 8601 date structure with only the date, but we could nest two time.Parses (one with time.RFC3339 and the other with "2006-01-02") or we could use dateparse.ParseAny.

edit: dateparse is also in go.mod.

> doe RFC3339 accept date only? If not, we should accept a date in RFC3339 structure, but with only the date.. It seems that `time.RFC3339` doesn't support an ISO 8601 date structure with only the date, but we could nest two `time.Parse`s (one with `time.RFC3339` and the other with `"2006-01-02"`) or we could use `dateparse.ParseAny`. edit: `dateparse` is also in `go.mod`.

@YakoYakoYokuYoku Sounds good to me to have "2006-01-02" and RFC3339 as fallback. I'd avoid another dependency

Scratch that, we already have dateparse 🙃 Lets go for dateparse.ParseLocal

~~@YakoYakoYokuYoku Sounds good to me to have "2006-01-02" and RFC3339 as fallback. I'd avoid another dependency~~ Scratch that, we already have dateparse 🙃 Lets go for `dateparse.ParseLocal`
YakoYakoYokuYoku added 1 commit 4 months ago
4416c6099a
Incorporate deadline functionality

Yeah I know that I'm using time.Parse here, but I would be thankful if the reviews point out other important things.

Yeah I know that I'm using `time.Parse` here, but I would be thankful if the reviews point out other important things.
noerw requested changes 4 months ago
modules/interact/milestone_create.go Outdated
)
// CreateMilestone interactively creates a milestone
func CreateMilestone(login *config.Login, owner, repo string, deadline *time.Time, state gitea.StateType) error {
noerw commented 4 months ago
Poster

No need for the deadline & state params: They will never be set anyway

No need for the deadline & state params: They will never be set anyway
6543 commented 4 months ago
Poster
Collaborator

didn't he add a flag for this?

didn't he add a flag for this?
noerw commented 4 months ago
Poster

Yeah, but interactive mode won't be enabled if there is a flag set..
We could ofcourse change the condition to go into interactive mode, if not all mandatory flags are provided, but idk if that's better

Yeah, but interactive mode won't be enabled if there is a flag set.. We could ofcourse change the condition to go into interactive mode, if not all mandatory flags are provided, but idk if that's better
6543 commented 4 months ago
Poster
Collaborator

no interactive should only if no flags are pressent - how would you integrate tea into scripts if it would ask you the whole time ...

no interactive should only if no flags are pressent - how would you integrate tea into scripts if it would ask you the whole time ...
6543 commented 4 months ago
Poster
Collaborator
-func CreateMilestone(login *config.Login, owner, repo string, deadline *time.Time, state gitea.StateType) error {
+func CreateMilestone(login *config.Login, owner, repo string) error {
```diff -func CreateMilestone(login *config.Login, owner, repo string, deadline *time.Time, state gitea.StateType) error { +func CreateMilestone(login *config.Login, owner, repo string) error { ```
YakoYakoYokuYoku marked this conversation as resolved
modules/interact/milestone_create.go Outdated
}
// deadline
promptI = &survey.Input{Message: "Milestone deadline [no due date]:"}
noerw commented 4 months ago
Poster

I'd put the date parsing inside a survey.Validator similar to promptRepoSlug(). This lets users iterate on wrong inputs, instead of failing the entire command

I'd put the date parsing inside a `survey.Validator` similar to `promptRepoSlug()`. This lets users iterate on wrong inputs, instead of failing the entire command

Gotcha, thx for pointing that out 👍

Gotcha, thx for pointing that out 👍
YakoYakoYokuYoku marked this conversation as resolved
6543 requested changes 4 months ago
cmd/milestones/create.go Outdated
if err != nil {
return err
if ctx.NumFlags() == 0 {
return interact.CreateMilestone(ctx.Login, ctx.Owner, ctx.Repo, deadline, state)
6543 commented 4 months ago
Poster
Collaborator
-		return interact.CreateMilestone(ctx.Login, ctx.Owner, ctx.Repo, deadline, state)
+		return interact.CreateMilestone(ctx.Login, ctx.Owner, ctx.Repo)
```diff - return interact.CreateMilestone(ctx.Login, ctx.Owner, ctx.Repo, deadline, state) + return interact.CreateMilestone(ctx.Login, ctx.Owner, ctx.Repo) ```
YakoYakoYokuYoku marked this conversation as resolved
modules/interact/milestone_create.go Outdated
// CreateMilestone interactively creates a milestone
func CreateMilestone(login *config.Login, owner, repo string, deadline *time.Time, state gitea.StateType) error {
var title, description, dueDate string
6543 commented 4 months ago
Poster
Collaborator
-	var title, description, dueDate string
+	var title, description, dueDate string
+	var deadline *time.Time
```diff - var title, description, dueDate string + var title, description, dueDate string + var deadline *time.Time ```
YakoYakoYokuYoku marked this conversation as resolved
modules/interact/milestone_create.go Outdated
title,
description,
deadline,
state)
6543 commented 4 months ago
Poster
Collaborator
-		state)
+		gitea.StateOpen)
```diff - state) + gitea.StateOpen) ```
YakoYakoYokuYoku marked this conversation as resolved
YakoYakoYokuYoku added 1 commit 4 months ago
2be9a124f2
Use dateparse and cleanup CreateMilestone task
YakoYakoYokuYoku requested review from noerw 4 months ago
YakoYakoYokuYoku requested review from 6543 4 months ago
noerw approved these changes 4 months ago
6543 approved these changes 4 months ago
6543 merged commit 43e9943757 into master 4 months ago
Collaborator

@YakoYakoYokuYoku nice work 👍

@YakoYakoYokuYoku nice work :+1:

You welcome too 👏

You welcome too 👏

Reviewers

noerw approved these changes 4 months ago
6543 approved these changes 4 months ago
continuous-integration/drone/pr Build is passing
The pull request has been merged as 43e9943757.
Sign in to join this conversation.
Loading…
There is no content yet.