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

Aggregate tailer tooltip host on instance number and tasks dropdown #1022

Conversation

@Calvinp
Copy link
Contributor

Calvinp commented May 3, 2016

This implements a tooltip showing the host and deploy id on the instance number in the aggregate tailer, and one showing only the host on the dropdown showing all the tasks.

@@ -227,6 +227,14 @@ class Utils
splits = taskId.split('-')
splits[splits.length - 3]

@getDeployIdFromTaskId: (taskId) ->

This comment has been minimized.

Copy link
@ssalinas

ssalinas May 3, 2016

Member

I think it might be a bit cleaner and more reusable to have a model for the TaskId. i.e. have something like a fromString method for the taskId model to set the attributes. Reusable where we need it, and you are then only calling the split once when you build the object instead of multiple times for the same string

@Calvinp Calvinp added the hs_staging label May 3, 2016
…, to avoid having to split the taskId over and over
@ssalinas ssalinas modified the milestone: 0.6.0 May 4, 2016
@Calvinp Calvinp added the hs_qa label May 4, 2016
@ssalinas ssalinas added the hs_stable label May 6, 2016
@ssalinas
Copy link
Member

ssalinas commented May 9, 2016

LGTM

@ssalinas ssalinas merged commit d018e93 into master May 9, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ssalinas ssalinas deleted the aggregate-tailer-tooltip-host-on-instance-count-and-maybe-tasks-dropdown branch May 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.