-
-
Notifications
You must be signed in to change notification settings - Fork 740
fix: a couple of fixes and improvements on task --init
#2433
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR improves the task --init
command by fixing taskfile detection logic and updating the default template. The changes ensure proper detection of existing taskfiles regardless of their extension and provide a better default template with YAML language server support.
- Changed default file extension from
.yml
to.yaml
for consistency - Fixed taskfile existence check to look for all supported taskfile names instead of just
Taskfile.yml
- Enhanced the default template with a description field and YAML language server configuration
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
taskfile/templates/default.yml | Removed old template with .yml extension |
taskfile/templates/default.yaml | Added new template with .yaml extension, description field, and language server config |
taskfile/taskfile.go | Made DefaultTaskfiles slice exported for reuse in init logic |
taskfile/node_file.go | Updated to use the exported DefaultTaskfiles slice |
internal/fsext/fs_test.go | Updated test to use .yaml extension |
init_test.go | Updated tests to use .yaml extension and improved cleanup with defer |
init.go | Implemented proper taskfile detection using all supported filenames and updated template embedding |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
taskfile/templates/default.yaml
Outdated
@@ -0,0 +1,15 @@ | |||
# https://taskfile.dev | |||
# | |||
# yaml-language-server: $schema=https://taskfile.dev/schema.json |
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.
I'm a bit unsure about this one. It's not actually needed for --init
because Taskfile.yaml
is recognized by default. Not having it makes it clean.
On the other hand, it's useful to inform users that they can do this to keep the YAML LSP working for non-standard names. This would be useful for non-standard names used by task -t foo.yaml
or Taskfiles added to includes
.
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.
Its 3 more lines to delete each time I use the --init command option ...
If someone wanted that, they could setup a template. Would be useful to document.
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.
Taskfiles are recognized by default, but only if they have one of the default Taskfile names. I often include files with other names like foo.yml
and the directives at the top are useful for instructing the YAML server. I don't think the link to taskfile.dev
is necessary, but I'd leave the YAML server instruction personally.
It's much easier for a user who knows what they're doing to remove a line than it is for a new user who has no idea what a YAML directive/schema is to add it.
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.
My two cents
I agree with @pd93
I think taskfile.dev
is not necessary, I'd even say it adds more noise than value. I’m also in favor of keeping the YAML server instruction.
Two days ago, I gave a talk about Task, I created the file with task --init
and I had to serch in our website to find the YAML server instruction
@pd93 @vmaerten This is a bit oppinated, so I want your opinions on this one. Feel free to disagree or suggest changes. This is the default Taskfile for years. Needed some love for a while. We can eventually consider changing it completely, but in that case we'll need to update the documentation as well. Perhaps the right moment for it would be on: |
2a4a107
to
4c9250a
Compare
I'm fine with most of this. I'm not convinced about |
# | ||
# yaml-language-server: $schema=https://taskfile.dev/schema.json | ||
|
||
version: '3' |
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.
We could consider setting this to the current version of the binary creating the file. This ensures that the same features will be available for the person initialising the file and the people reading it.
It would also encourage the use of this feature which I think is underused. Most people just put 3
despite requiring a much more recent version.
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.
This is an interesting idea.
Would we want to add as 3.45.4
or 3.45
?
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.
3.45
is probably enough since we only add features on minor revisions anyway. The chances are that anyone getting a message to upgrade will immediately install the latest version instead of the minimum version anyway.
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.
A different perspective: unless a taskfile needs a specific version, then doing this will become an annoyance when people are working/sharing/using different versions of task/taskfiles. We don't use the feature because its not very useful for us ... the version of Task in use is already managed, which is sufficient.
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.
The version key is not intended to dictate a specific version. It is a minimum version required for the features in the Taskfile. The vast majority of Taskfiles will require a minimum version since they'll use features introduced after v3.0.0. For instance, any Taskfile that uses aliases would need to have a minimum version of 3.17.
This is extremely useful as instead of receiving a confusing error when they use a version of Task that is too old, they'll receive a message that tells them to update.
I think the main reason this is underused is because it didn't work this way until v3.34 and is underdocumented. None of our examples/tests use it and the tool itself doesn't advertise the feature. Adding this to the --init
flag is analogous to go mod init
adding the current version of Go
to the file.
6ae93dc
to
8f9c99d
Compare
* Fixed check for an existing Taskfile: look for all possibilities, and not only `Taskfile.yml` specifically. * Added a description (`desc`) to the `default` task. Important to at least `task --list` work by default (a core feature).
8f9c99d
to
550d730
Compare
task --init
task --init
OK, I reverted the default extension change and the language server comment. Still kept the addition of |
@andreynering here is an idea, perhaps you want an ENVAR/taskrc-file to allow setting of default taskfile names, which if present overrides this list: Line 15 in 4e84c6b
Then, to get the taskfile name to use with the --init option, take the first item from that list.
|
@trulede You can already change the name of the created file using the task --init --taskfile foo.yml |
Taskfile.yml
specifically.desc
) to thedefault
task. Important to at lesttask --list
work by default (a core feature).Changed default extension from.yml
to.yaml
.Addedyaml-language-server
comment.