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

Don't show delegating tasks in summary #639

Closed
gep13 opened this issue Jan 14, 2016 · 21 comments
Closed

Don't show delegating tasks in summary #639

gep13 opened this issue Jan 14, 2016 · 21 comments
Milestone

Comments

@gep13
Copy link
Member

gep13 commented Jan 14, 2016

i.e. Tasks that are only used to tie together other tasks. If there is no action associated with the Task, don't show it in the summary.

@patriksvensson
Copy link
Member

👍

@RichiCoder1
Copy link
Contributor

Or show it in a non-tasky way to may it clear it's a summary or branch task.

@ErikSchierboom
Copy link
Contributor

How would you like the delegating task to be displayed? In grey, just like skipped tasks? And what should be the displayed text?

@ErikSchierboom
Copy link
Contributor

In a previous PR, I add a Skipped property to the CakeReportEntry class. However, I now think that should really have been an enumeration. That way, we can have an enumeration that specifies the executed task's status: Executed, Skipped or Delegated. Should I replace the boolean property with an enumeration, or create an additional boolean property?

@patriksvensson
Copy link
Member

@ErikSchierboom I agree with your thinking. An enum would be much cleaner. Not sure yet how the delegating task should be displayed. Not against graying it out, but the total won't match the sum of the tasks (since tasks that "delegate" could take some milliseconds). Not sure if this really matters. Maybe we can write the time for the delegated task but gray it out?

What do you think?

(ping @gep13 @devlead)

@devlead
Copy link
Member

devlead commented Jan 24, 2016

Gray is nice, but as build server logs are just text might not clear enough. Maybe just report on diagnostic / verbose?

@patriksvensson
Copy link
Member

@devlead Reports are not written to the log. They're written to the console.

@devlead
Copy link
Member

devlead commented Jan 24, 2016

Usually build servers display console output as log right?

@patriksvensson
Copy link
Member

@devlead Yes, they are. But we're writing the report using IConsole (and not ICakeLog) beacuse we want to do "pretty printing" of the summary (using colors).

@devlead
Copy link
Member

devlead commented Jan 24, 2016

I understand that, colors rarely preserved on CI, so just an esthetic when running local then?

@patriksvensson
Copy link
Member

@devlead Yes, exactly. So the question is if we:

  1. Exclude "delegating" tasks (so they won't show up in either log)
  2. Grey them out
  3. Grey them out and add some description that tells the user that they just branched of to other tasks.

@devlead
Copy link
Member

devlead commented Jan 24, 2016

Ok, I would vote hide by verbosity so they're shown if diagnostic.

@patriksvensson
Copy link
Member

@devlead That is a good idea.

@ErikSchierboom What do you think about that?

@ErikSchierboom
Copy link
Contributor

Hiding by verbosity seems like a good idea! Should I only show the duration in gray, or also the task itself?

@devlead
Copy link
Member

devlead commented Jan 24, 2016

I think both. @patriksvensson ?

@patriksvensson
Copy link
Member

@ErikSchierboom @devlead Maybe we need to think about this some more. I think we might get issues posted for this otherwise like BUG: Some tasks is not shown unless using diagnostic verbosity.

@patriksvensson
Copy link
Member

@ErikSchierboom @devlead Maybe we need to add a description like we did with Skipped.

@patriksvensson
Copy link
Member

OK, maybe we should just go with the one we agreed on and figure this out later? It's probably good 😄

@ErikSchierboom
Copy link
Contributor

But what is the one we agreed on? The one with the verbosity? Or with the description?

@patriksvensson
Copy link
Member

Sorry, I meant the one with verbosity.

@ErikSchierboom
Copy link
Contributor

I've create a PR to implement this functionality. I believe that issue #123 is a duplicate of this one by the way.

gep13 added a commit that referenced this issue Jan 31, 2016
@gep13 gep13 added this to the v0.9.0 milestone Jan 31, 2016
@gep13 gep13 closed this as completed Jan 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants