Skip to content

Conversation

@krzyzacy
Copy link
Member

Save some unnecessary bleeding 😫

/assign @pipejakob

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 17, 2017
Copy link
Contributor

@pipejakob pipejakob left a comment

Choose a reason for hiding this comment

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

LGTM. No blocking comments, just a few minor suggestions.


import kubernetes_e2e

FAKE_SCM = 'STABLE_BUILD_GIT_COMMIT 599539dc0b99976fda0f326f4ce47e93ec07217c\n' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick: There's a lot of information in here besides the SCM, so calling it FAKE_WORKSPACE_STATUS seems more appropriate. Same goes for fake_output_scm -> fake_output_workspace_status.

Copy link
Member Author

Choose a reason for hiding this comment

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

done.


FAKE_SCM = 'STABLE_BUILD_GIT_COMMIT 599539dc0b99976fda0f326f4ce47e93ec07217c\n' \
'STABLE_BUILD_SCM_STATUS clean\n' \
'STABLE_BUILD_SCM_REVISION v1.7.0-alpha.0.1320+599539dc0b9997\n' \
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you have real values here, it would be great if the test actually asserted the expected value of the SCM_REVISION that gets extracted (to make sure that the regex doesn't accidentally pick up leading/trailing whitespace, newlines, etc).

Copy link
Member Author

Choose a reason for hiding this comment

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

after fighting with the bot for a while, done.

@krzyzacy krzyzacy force-pushed the kubeadm-test branch 3 times, most recently from 9310b9d to c624553 Compare March 17, 2017 22:01
@krzyzacy
Copy link
Member Author

merge to get couple runs before EoD

@krzyzacy krzyzacy merged commit eb3b7fd into kubernetes:master Mar 17, 2017
@krzyzacy krzyzacy deleted the kubeadm-test branch March 17, 2017 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants