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

Show Dry Run Result with tabs #1371

Merged
merged 4 commits into from Mar 27, 2016
Merged

Show Dry Run Result with tabs #1371

merged 4 commits into from Mar 27, 2016

Conversation

thiagotalma
Copy link
Contributor

For better organization and viewing the results.

@dsander
Copy link
Collaborator

dsander commented Mar 25, 2016

Nice, I like it! Not sure about the order of the tabs though, wouldn't one want to look at the generated events first and switch to the log only if something went wrong?

@knu
Copy link
Member

knu commented Mar 25, 2016

Cool, looks good! I'd agree with @dsander about the order. When I implemented it I thought the log should be the main content when everything was to be shown in one page, but once separated, created events should probably be in the primary tab because they are the main effect of a run.

@thiagotalma
Copy link
Contributor Author

@dsander @knu
Done!

@thiagotalma
Copy link
Contributor Author

@dsander @knu @cantino
What do you think to remove the date/time of the logs?
They take up much space on the screen and do not matter, since the execution was done manually.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.753% when pulling a24d1bc on thiagotalma:dry-tabs into 0755451 on cantino:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.753% when pulling 84fd636 on thiagotalma:dry-tabs into 0755451 on cantino:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 90.781% when pulling 92064a0 on thiagotalma:dry-tabs into 0755451 on cantino:master.

@dsander
Copy link
Collaborator

dsander commented Mar 26, 2016

@thiagotalma I think it is a good idea to remove the timestamps.

@knu
Copy link
Member

knu commented Mar 26, 2016

I think elapsed times in second precision would be great.

@knu
Copy link
Member

knu commented Mar 26, 2016

And it could be a separate PR.

""",
body: (body) ->
$(body).
find('.agent-dry-run-log').text(json.log).end().
find('.agent-dry-run-events').text(json.events).end().
find('.agent-dry-run-memory').text(json.memory)
active = if json.events.match(/^\[?\s*\]?$/) then 'tabLog' else 'tabEvents'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this line do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defines the tab will be activated according to checking whether the events are empty. If empty, the Logs tab is activated.

@cantino
Copy link
Member

cantino commented Mar 26, 2016

Nice improvement!

@cantino cantino merged commit d13edc2 into huginn:master Mar 27, 2016
@cantino
Copy link
Member

cantino commented Mar 27, 2016

Looks good to me, thanks! Merging.

@thiagotalma thiagotalma deleted the dry-tabs branch March 27, 2016 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants