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

Fixes #18754: Add File content page #6642

Merged
merged 1 commit into from Mar 17, 2017

Conversation

Projects
None yet
4 participants
@ehelms
Copy link
Member

commented Mar 1, 2017

Requires -- Katello/bastion#174

@mention-bot

This comment has been minimized.

Copy link

commented Mar 1, 2017

@ehelms, thanks for your PR! By analyzing the history of the files in this pull request, we identified @waldenraines, @jlsherrill and @bbuckingham to be potential reviewers.

...on_katello/app/assets/javascripts/bastion_katello/files/details/views/file-repositories.html Outdated
<tbody>
<tr bst-table-row ng-repeat="repository in table.rows">
<td bst-table-cell>
<a ng-href="/products/{{ repository.product.id }}/repositories/{{ repository.library_instance_id || repository.id }}">

This comment has been minimized.

Copy link
@waldenraines

waldenraines Mar 7, 2017

Member

Can you use ui-sref here?

...on_katello/app/assets/javascripts/bastion_katello/files/details/views/file-repositories.html Outdated
Not Synced
</span>
<span ng-hide="repository.last_sync == null">
<a href="/foreman_tasks/tasks/{{repository.last_sync.id}}">{{ repository.last_sync.result | capitalize}}</a>

This comment has been minimized.

Copy link
@waldenraines

waldenraines Mar 7, 2017

Member

Should this be ng-href?

engines/bastion_katello/app/assets/javascripts/bastion_katello/files/views/files.html Outdated
<table class="table table-striped table-bordered" ng-class="{'table-mask': table.working}">
<thead>
<tr bst-table-head>
<th bst-table-column="name">{{ "Name" | translate }}</th>

This comment has been minimized.

Copy link
@waldenraines

waldenraines Mar 7, 2017

Member

Can you use the directive form of translate here instead of the filter?

This comment has been minimized.

Copy link
@ehelms

ehelms Mar 17, 2017

Author Member

This is the form we use everywhere else for table headers, IIRC due to how the directives clash on transclusion.

This comment has been minimized.

Copy link
@waldenraines

waldenraines Mar 17, 2017

Member

Oh yeah, that's right. I've been updating to this form everywhere when I was converting from nutupane:

<th bst-table-column="name">
  <span translate>Name</span>
</th>
@theforeman-bot

This comment has been minimized.

Copy link

commented Mar 17, 2017

There were the following issues with the commit message:

  • 08c7bd1ada73f0136ab0b07b8f3fc9f4025f2431 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@ehelms

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2017

@waldenraines addresed all comments

@waldenraines

This comment has been minimized.

Copy link
Member

commented Mar 17, 2017

Should yum filters within the same content view show up in the file filters list?

@ehelms ehelms force-pushed the ehelms:fixes-18754 branch Mar 17, 2017

@ehelms ehelms force-pushed the ehelms:fixes-18754 branch to 930ba4c Mar 17, 2017

@ehelms

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2017

@waldenraines Updated

@ehelms ehelms merged commit a216a1a into Katello:master Mar 17, 2017

2 checks passed

default Job result: SUCCESS
Details
hound No violations found. Woof!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.