Skip to content

add conditions to build api#52

Merged
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
bparees:buildconditions
Jan 7, 2020
Merged

add conditions to build api#52
openshift-merge-robot merged 2 commits intoopenshift:masterfrom
bparees:buildconditions

Conversation

@bparees
Copy link
Copy Markdown
Contributor

@bparees bparees commented Dec 3, 2019

implementation of https://jira.coreos.com/browse/DEVEXP-95

api updates: openshift/api#532
client-go updates(using temporary bump from bparees fork): openshift/client-go#128
openshift-apiserver updates(using temporary bump from bparees fork): #52
origin(e2e test, using temporary bump from bparees fork) updates: openshift/origin#24254

@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 3, 2019

/assign @adambkaplan

@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 3, 2019

@adambkaplan Once this PR goes green, we can merge openshift/api#532 and openshift/client-go#128, then i'll come back and update this PR to use a proper bump. Then once it merges, we can retest/merge openshift/origin#24254

@bparees bparees added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 3, 2019
@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 4, 2019

/retest

Copy link
Copy Markdown
Contributor

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

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

@bparees minor nits on the timestamps, otherwise looks right

Type: buildv1.BuildConditionType(buildv1.BuildPhaseNew),
Status: corev1.ConditionTrue,
LastUpdateTime: metav1.Now(),
LastTransitionTime: metav1.Now(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit - use same object for LastUpdateTime and LastTransitionTime

Type: buildv1.BuildConditionType(buildv1.BuildPhaseNew),
Status: corev1.ConditionTrue,
LastUpdateTime: metav1.Now(),
LastTransitionTime: metav1.Now(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit - use same object for timestamps

if c.Status != kapi.ConditionFalse {
build.Status.Conditions[i].Status = kapi.ConditionFalse
build.Status.Conditions[i].LastTransitionTime = metav1.Now()
build.Status.Conditions[i].LastUpdateTime = metav1.Now()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit - use same object for timestamps

Type: buildapi.BuildConditionType(build.Status.Phase),
Status: kapi.ConditionTrue,
LastUpdateTime: metav1.Now(),
LastTransitionTime: metav1.Now(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit - use same object for timestamps

@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 9, 2019

@adambkaplan updated. if you can lgtm openshift/api#532 i'll start the process of merging/bumping across the repos.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 13, 2019
@openshift-ci-robot openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 13, 2019
@bparees bparees changed the title [DO_NOT_MERGE] add conditions to build api add conditions to build api Dec 13, 2019
@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2019
@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 13, 2019

/hold cancel

@adambkaplan this is ready for re-review/merge.

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 13, 2019
@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 16, 2019

/retest

@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 18, 2019

/test cmd

2 similar comments
@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 18, 2019

/test cmd

@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Dec 20, 2019

/test cmd

@bparees
Copy link
Copy Markdown
Contributor Author

bparees commented Jan 6, 2020

/retest

@adambkaplan ptal

Copy link
Copy Markdown
Contributor

@adambkaplan adambkaplan left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 7, 2020
@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, bparees

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit b6c37a5 into openshift:master Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants