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

renamed text job to phrases for easier comprehension #85

Closed
wants to merge 4 commits into from

Conversation

mackenza
Copy link
Contributor

when I first looked at Kitto I looked at the text widget and didn't quite grok the relationship between a widget, a job and the dashboard. Part of this was because the job was named text as was the widget (well Text I guess technically).

I simply renamed text.exs to phrases.exs and changed the installer template so now the text widget on the sample dashboard is similar to the other widgets where the widget name is different than the job name (`data-source="phrases" in the dashboard).

It's a small change but I think could help.

@mackenza
Copy link
Contributor Author

some more background can be found on Gitter where @davejlong and I discussed this topic. Thanks Dave!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 86.88% when pulling 758d345 on mackenza:rename-text-job into 261f508 on kittoframework:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 86.88% when pulling 758d345 on mackenza:rename-text-job into 261f508 on kittoframework:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 86.88% when pulling 758d345 on mackenza:rename-text-job into 261f508 on kittoframework:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.9%) to 86.88% when pulling 758d345 on mackenza:rename-text-job into 261f508 on kittoframework:master.

@mackenza
Copy link
Contributor Author

@zorbash if you want me to clean this up with a new PR let me know. I tested my initial changes but it was working because I hadn't changed the dashboard and job to match. Fixed now.

@coveralls
Copy link

coveralls commented Jan 12, 2017

Coverage Status

Coverage increased (+0.9%) to 86.88% when pulling 08fddbc on mackenza:rename-text-job into 261f508 on kittoframework:master.

@@ -3,5 +3,5 @@ use Kitto.Job.DSL
job :text, every: {4, :seconds} do
Copy link
Member

@zorbash zorbash Jan 12, 2017

Choose a reason for hiding this comment

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

Rename the job to :phrases and remove the 1st argument which isn't required anymore from broadcast.

job :phrases, every: {4, :seconds} do
  # ...
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@zorbash
Copy link
Member

zorbash commented Jan 12, 2017

Merged as 09bc604
Thanks @mackenza

@zorbash zorbash closed this Jan 12, 2017
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

3 participants