Skip to content
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

Feature 664 group error only #1022

Merged
merged 5 commits into from Mar 9, 2023

Conversation

jaedle
Copy link
Contributor

@jaedle jaedle commented Feb 25, 2023

Implementation idea for #664.

Describe your changes

Usage

version: '3'

silent: true

output:
  group:
    error_only: true

tasks:
  passing: echo 'passing-output'
  failing: echo 'failing-output' && exit 1
> task passing
(no output)
> task failing
failing-output
task: Failed to run task "failing": exit status 1

Implementation

Enrich Group output by an error_only flag which should swallow the output (stdout + stderr) if any external command did not have a non zero exit code. By default this flag is disabled.

backwards comapatibility

Changes should not interfere with task files created in previous versions as the default behavior is not changed.

What's already done

  • high level tests (task_test.go)
  • tests for output writer
  • code style (fmt & lint)
  • documentation
  • schema definition

I would love to have some feedback if this goes into the right direction. If so, I would love to continue working on it.

@jaedle jaedle force-pushed the feature-664-group-error-only branch from 0f6377f to 4fa3b65 Compare February 25, 2023 12:37
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.

Hi @jaedle!

Thanks for working on this!

This is in the right direction, but I commented a few changes before this can be merged.

Other than the documentation, something else missing the updating the schema.json to reflect this change. If you don't want to do that yourself, let us know and we'll do once merged.

internal/output/output.go Outdated Show resolved Hide resolved
internal/output/group.go Outdated Show resolved Hide resolved
task.go Outdated Show resolved Hide resolved
task.go Outdated Show resolved Hide resolved
@jaedle
Copy link
Contributor Author

jaedle commented Mar 2, 2023

Hi @andreynering,

thanks for your feedback. I will try to catch up on your review comments on the weekend.

@jaedle jaedle closed this Mar 4, 2023
@jaedle jaedle force-pushed the feature-664-group-error-only branch from 3e4a157 to a36b1b9 Compare March 4, 2023 09:42
@jaedle jaedle reopened this Mar 4, 2023
@jaedle
Copy link
Contributor Author

jaedle commented Mar 4, 2023

Hey @andreynering,

as you have suggested a different approach (add a flag to output group instead of a new output group_error_only), it was way easier to start the implementation from scratch. I am sorry if this creates more effort on your side, but this was way cleaner.

I have updated the issue to reflect the new implementation idea.

I tried to implement all your review comments. If there is still anything missing on this pr, feel free leave a note.

@jaedle jaedle marked this pull request as ready for review March 4, 2023 10:32
@andreynering andreynering merged commit 88d644a into go-task:master Mar 9, 2023
andreynering added a commit that referenced this pull request Mar 9, 2023
@andreynering
Copy link
Member

Thanks for your contribution @jaedle, it's very appreciated!

@jaedle jaedle deleted the feature-664-group-error-only branch March 12, 2023 10:16
@pd93 pd93 mentioned this pull request Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants