-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Builds page - group builds by build config and show more details for build configs #1134
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
Conversation
assets/app/views/builds.html
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is possible to have output.dockerImageReference instead of output.to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm not seeing output.dockerImageReference coming back from the API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's here eventually (although deprecated) you can look into ImageTag and Registry but DockerImageReference should take precedence over the two (see latter lines in the link).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@csrwng can you send me a sample template that has a build that would produce dockerImageReference in the API output, all the examples under the sample-app do not return dockerImageReference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sample build config is here: https://gist.github.com/csrwng/61628e83581cf077f772
Sample build is here: https://gist.github.com/csrwng/8a2b9fa5e2a67ad65b3b
|
@yin doesn't need to be part of your existing PR, but make a note to come back and revisit this after adding the From references because i think it affects some of this code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix comment
72244c9 to
e672e6d
Compare
|
Updated based on review feedback. Updated screenshots. |
e672e6d to
31b1ae3
Compare
31b1ae3 to
61c974a
Compare
|
and now with builds under the same build config being sorted by date (most recent first) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double check in different browsers to make sure this works on keyboard focus and on click. might need to listen for both
|
LGTM |
|
[merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1037/) (Image: devenv-fedora_891) |
|
Evaluated for origin up to 61c974a |
Merged by openshift-bot


No description provided.