go-task / task

A task runner / simpler Make alternative written in Go
https://taskfile.dev
MIT License
11.56k stars 616 forks source link

Make the taskfile in include required #1902

Open semihbkgr opened 3 weeks ago

semihbkgr commented 3 weeks ago

fixes #1881

This makes the taskfile field within include required, disallowing null (unspecified) or empty values, even if inlined.

I’d like to clarify two points, @pd93:

1- Would it be preferable to return a TaskfileDecodeError in the UnmarshalYAML function when parsing the yaml into Include? This approach could make the error message more descriptive.

Screenshot from 2024-11-02 22-51-17

However, I aligned this error with the existing one we return when the Taskfile is not existing ("no such file or directory"), which is why the error is returned after the parsing step.

2- Should we differentiate between null and empty string values, or should we treat them the same and return an error for both?

pd93 commented 3 weeks ago

1- Would it be preferable to return a TaskfileDecodeError in the UnmarshalYAML function when parsing the yaml into Include? This approach could make the error message more descriptive.

However, I aligned this error with the existing one we return when the Taskfile is not existing ("no such file or directory"), which is why the error is returned after the parsing step.

The distinction here is whether or not taskfile: is required in the schema or whether it is valid to be empty in the schema, but causes an error when processing.

Opinion: I think we should error in the UnmarshalYAML method and return a TaskfileDecodeError and we should update the schema.json to mark taskfile: as a required field when using an include: object. I'm not sure if jsonschema can specify non-empty on a scalar value? but if it can, we should do that too. @andreynering @vmaerten feel free to weigh-in.

2- Should we differentiate between null and empty string values, or should we treat them the same and return an error for both?

I can't see a situation where an empty string is useful.

semihbkgr commented 1 week ago

@pd93 We can use "minLength": 1 in the json schema to ensure the taskfile value is not empty.