Skip to content

Conversation

@jaedle
Copy link
Contributor

@jaedle jaedle commented Feb 24, 2019

Adding a --summary flag for task to get more a summary of the task

See #107

New field in task called summary

version: '2'
tasks:
  release:
    deps: [build]
    summary: |
      Release your project to github

      It will build your project before starting the release it.
      Please make sure that you have set GITHUB_TOKEN before starting.
    cmds:
      - your-release-tool
  build:
    cmds:
      - your-build-tool

Output for task --summary release:

task: release

Release your project to github

It will build your project before starting the release it.
Please make sure that you have set GITHUB_TOKEN before starting.

dependencies:
 - build

commands:
 - your-release-tool

Behaviour:

  • --summary will not execute the task
  • --summary will display the summary of a task (see above)
  • --summary will use description as fallback, if summary does not exist
  • --summary will output a warning (or information), if no description/summary is present
  • --summary will output the dependencies if there are any
  • --summary will output the commands if there are any

Tests:

  • Because there are a lot of edge-cases I decided to extract out a package (summary), which is completly unit tested with above named edge cases.
  • There is an integration like test in task_test.go

Documentation is present for that feature

@jaedle jaedle changed the title Add help informations for a task #107 Add help informations for a task Feb 24, 2019
@jaedle
Copy link
Contributor Author

jaedle commented Feb 24, 2019

Usage:
task --details some-task

There are still some questions to me:
What should happen, if there is no detailed description for this task?

  • Outputting an error message makes perfect sense for me.
  • Should there be a non-zero exit value? I would guess no..

What should happen, if there are given multiple tasks?
Actually it shows just the description for the first one..

@tonobo
Copy link
Contributor

tonobo commented Feb 24, 2019

Wouldn't it be cooler to have kind of inspect/describe command, which shows long description and the whole command specification?

@jaedle jaedle marked this pull request as ready for review February 24, 2019 11:01
@jaedle jaedle changed the title Add help informations for a task Display detailed information about a task Feb 24, 2019
@jaedle
Copy link
Contributor Author

jaedle commented Feb 24, 2019

Wouldn't it be cooler to have kind of inspect/describe command, which shows long description and the whole command specification?

I think there is a difference. Sometimes it makes sense, if you have preconditions or something like that, to document it. I think for that a details section makes perfect sense.
For other purposes I think your suggestion is also very good - but maybe in a different use case.

@tonobo
Copy link
Contributor

tonobo commented Feb 24, 2019

I meant this in addition to your newly introduced keyword, it's pretty cool to give tasks a more detailed description. But the details context could print also the whole task in detail.

@jaedle
Copy link
Contributor Author

jaedle commented Feb 24, 2019

Could you propose a sample output for this?

@tonobo
Copy link
Contributor

tonobo commented Feb 24, 2019

I don't know maybe something like this ?

task: build

Pretty long description comes here Lorem ipsum dolor sit amet, 
consectetur adipiscing elit, sed do eiusmod tempor incididunt ut 
labore et dolore magna aliqua.

Dependencies:
  - lint
  - format

Commands:
  - sh -c "go build ..."

Copy link
Member

@andreynering andreynering left a comment

Choose a reason for hiding this comment

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

Hey @jaedle, great work with the code, tests and documentation! 👏 👏👏

Thanks for spending the time on work on this. I did just a few comments, but once addresses this is good to go.

andreynering and others added 6 commits March 3, 2019 18:56
Co-Authored-By: jaedle <32975714+jaedle@users.noreply.github.com>
Co-Authored-By: jaedle <32975714+jaedle@users.noreply.github.com>
Co-Authored-By: jaedle <32975714+jaedle@users.noreply.github.com>
Co-Authored-By: jaedle <32975714+jaedle@users.noreply.github.com>
Co-Authored-By: jaedle <32975714+jaedle@users.noreply.github.com>
Co-Authored-By: jaedle <32975714+jaedle@users.noreply.github.com>
@jaedle
Copy link
Contributor Author

jaedle commented Mar 3, 2019

Hey @jaedle, great work with the code, tests and documentation! 👏 👏👏

Thanks for spending the time on work on this. I did just a few comments, but once addresses this is good to go.

Hey @andreynering,

thanks for the review. I‘m quite new to golang, so I really appreciate it! Fixes are comming next week 😊

task.go Outdated
}

if e.Summary {
summary.PrintAll(e.Logger, e.Taskfile, calls)
Copy link
Contributor Author

@jaedle jaedle Mar 4, 2019

Choose a reason for hiding this comment

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

I've been thinking hard how to do this.
I think the task component (high level abstraction, only delegates to other components) should not care about a low level detail like spacing for the output of the summary.
To fix this, I added a PrintTasks Method for the summary-package, which deals with it and is unit-tested.

@jaedle
Copy link
Contributor Author

jaedle commented Mar 4, 2019

I really hope this is good to go! :)

@andreynering
Copy link
Member

@jaedle Thanks again!

If I can give you a friendly tip, next time try keeping a smaller number of commits.

@andreynering andreynering merged commit d516b23 into go-task:master Mar 5, 2019
@jaedle
Copy link
Contributor Author

jaedle commented Mar 5, 2019

@jaedle Thanks again!

If I can give you a friendly tip, next time try keeping a smaller number of commits.

Thanks for the recommandation. I usally tend to squash merge, if you don‘t, I will squash the branch before raising the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants