Skip to content

Conversation

@jwforres
Copy link
Member

Fixes #2975

@jwforres jwforres force-pushed the fix_trigger_build_association branch from a92256f to a7bdb6b Compare November 10, 2015 18:49
@jwforres
Copy link
Member Author

@spadgett thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Assume you didn't mean to put the namespace attr on the closing tag :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope I did not :)

@spadgett
Copy link
Member

This certainly looks cleaner.

@jwforres jwforres force-pushed the fix_trigger_build_association branch 2 times, most recently from 2c8c10a to c272112 Compare November 10, 2015 19:31
@jwforres jwforres changed the title [WIP] Change how we store association between builds and ICTs on DCs for the console overview Change how we store association between builds and ICTs on DCs for the console overview Nov 10, 2015
@jwforres
Copy link
Member Author

@spadgett updated

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can use the <status-icon status="build.status.phase"> directive here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep will clean this up while i'm in here

@spadgett
Copy link
Member

Should we filter the builds that aren't recent from the map so we don't have to iterate over every build every digest loop in _triggers.html?

@jwforres jwforres force-pushed the fix_trigger_build_association branch 2 times, most recently from 2176287 to 5157bd4 Compare November 10, 2015 21:48
Copy link
Member Author

Choose a reason for hiding this comment

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

would like to add a css style for this before merging (class = divider), will do that tmrw

Copy link
Member

Choose a reason for hiding this comment

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

I think we might use that class name already for horizontal dividers

On Tuesday, November 10, 2015, Jessica Forrester [email protected]
wrote:

In assets/app/views/_triggers.html
#5836 (comment):

       </span>
     </span>
  •  </span>
    
  •  <a
    
  •    ng-if="!!['New', 'Pending'].indexOf(build.status.phase)"
    
  •    ng-href="project/{{build.metadata.namespace}}/browse/builds/{{build.metadata.labels.buildconfig}}/{{build.metadata.name}}?tab=logs">
    
  •    View Log
    
  •  </a>
    
  •  <a ng-hide="build | isIncompleteBuild" style="margin-left: 5px;" href="" ng-click="hideBuild(build)">Dismiss</a>
    
  •    <a
    
  •      ng-if="!!['New', 'Pending'].indexOf(build.status.phase)"
    
  •      ng-href="project/{{build.metadata.namespace}}/browse/builds/{{build.metadata.labels.buildconfig}}/{{build.metadata.name}}?tab=logs">
    
  •      View Log
    
  •    </a>
    
  •    <span ng-show="!!['New', 'Pending'].indexOf(build.status.phase) && !(build | isIncompleteBuild)" style="color: #999;">|</span>
    

would like to add a css style for this before merging (class = divider),
will do that tmrw


Reply to this email directly or view it on GitHub
https://github.com/openshift/origin/pull/5836/files#r44470315.

@jwforres jwforres force-pushed the fix_trigger_build_association branch from 5157bd4 to 58b19f3 Compare November 11, 2015 16:47
@jwforres
Copy link
Member Author

went with action-divider for now

updated, LGTY @spadgett ?

@spadgett
Copy link
Member

LGTM

@jwforres jwforres added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2015
@jwforres
Copy link
Member Author

[merge]

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7101/) (Image: devenv-rhel7_2701)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 58b19f3

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 58b19f3

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin/7101/)

openshift-bot pushed a commit that referenced this pull request Nov 13, 2015
@openshift-bot openshift-bot merged commit 6b2ef9b into openshift:master Nov 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/web lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants