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

#69 add job name in queue list #74

Merged
merged 4 commits into from
Dec 5, 2017
Merged

Conversation

MaximeMaillet
Copy link
Contributor

No description provided.

@eeVoskos
Copy link
Contributor

eeVoskos commented Dec 5, 2017

I was about to fork the project and add a job name in the UI, when I saw this PR! +1 for the attempt, although I'd change the code a bit.

You need to check if a job.name exists, as it is not a standard in bull. Or, even better, I'd add the name field in the job's data, not the job object itself.

So, I'd change the h4 row to something like this:

<h4 class="header-collapse"><span class="label label-default">{{ this.id }}</span>
  {{#if this.data.name}}
    <span style="margin-left: 8px">{{ this.data.name }}</span>
  {{/if}}
</h4>

@eeVoskos
Copy link
Contributor

eeVoskos commented Dec 5, 2017

@randallm @bradvogel would you be kind enough to accept this PR?!

{{#if this.data.name}}
<span style="margin-left: 8px">{{ this.data.name }}</span>
{{else if this.name}}
<span style="margin-left: 8px">{{ this.name }}</span>
Copy link
Member

Choose a reason for hiding this comment

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

The indentation here is slightly off.

@skeggse
Copy link
Member

skeggse commented Dec 5, 2017

Other than that one nit, this looks good to me. Works as advertised.

image

@MaximeMaillet
Copy link
Contributor Author

@skeggse Go ;)
Thanks

@skeggse skeggse merged commit 81c11fa into bee-queue:master Dec 5, 2017
@skeggse
Copy link
Member

skeggse commented Dec 5, 2017

released in v2.2.2

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.

3 participants