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

Typo fixes #6089

Merged
merged 1 commit into from Dec 1, 2015
Merged

Typo fixes #6089

merged 1 commit into from Dec 1, 2015

Conversation

jhadvig
Copy link
Member

@jhadvig jhadvig commented Nov 25, 2015

@Kargakis PTAL

@0xmichalis
Copy link
Contributor

both forms are correct I think

@@ -55,7 +55,7 @@ type BuildStatus struct {
Phase BuildPhase `json:"phase" description:"observed point in the build lifecycle"`

// Cancelled describes if a cancelling event was triggered for the build.
Cancelled bool `json:"cancelled,omitempty" description:"describes if a canceling event was triggered for the build"`
Cancelled bool `json:"cancelled,omitempty" description:"describes if a cancelling event was triggered for the build"`
Copy link
Contributor

Choose a reason for hiding this comment

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

You most probably need to update swagger for this change

./hack/update-generated-swagger-spec.sh

Copy link
Member

Choose a reason for hiding this comment

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

same thing... should be "cancel" not "cancelling"

@jhadvig
Copy link
Member Author

jhadvig commented Nov 25, 2015

more or less, but we use the double L in our code + in the printed messages

@jhadvig
Copy link
Member Author

jhadvig commented Nov 25, 2015

@Kargakis swagger updated. Thx for noticing :)

@0xmichalis
Copy link
Contributor

more or less, but we use the double L in our code + in the printed messages

ok, if it's decided between a {special,shady} committee behind closed doors then it looks good to me:)

cc: @fabianofranz @jwforres

@@ -15305,7 +15305,7 @@
},
"cancelled": {
"type": "boolean",
"description": "describes if a canceling event was triggered for the build"
"description": "describes if a cancelling event was triggered for the build"
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be "describes if a cancel event was triggered for the build"

@jwforres
Copy link
Member

and 👍 for consistency for single vs double L.... even though double L looks very wrong to me, there is no way we are getting the api enumeration value changed at this point, and the single L is only accepted in american english anyway - http://grammarist.com/spelling/cancel/

@jhadvig
Copy link
Member Author

jhadvig commented Nov 25, 2015

@jwforres to be honest I was surprised myself that the double L version was the "more" correct one. Also I've changed the description as suggested. Thanks

@jhadvig
Copy link
Member Author

jhadvig commented Nov 30, 2015

@jwforres is there anything else tbd ?

@jwforres
Copy link
Member

jwforres commented Dec 1, 2015

[merge]

@jhadvig
Copy link
Member Author

jhadvig commented Dec 1, 2015

@jwforres please re-merge. Failed due to flaky tests.

@jwforres
Copy link
Member

jwforres commented Dec 1, 2015

opened flake issue #6149

[merge]

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to ae4e36b

@openshift-bot
Copy link
Contributor

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

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4201/) (Image: devenv-rhel7_2848)

@jwforres
Copy link
Member

jwforres commented Dec 1, 2015

[merge]

failure added to #6121

On Tue, Dec 1, 2015 at 1:05 PM, OpenShift Bot notifications@github.com
wrote:

continuous-integration/openshift-jenkins/merge FAILURE (
https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4199/
)


Reply to this email directly or view it on GitHub
#6089 (comment).

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to ae4e36b

openshift-bot pushed a commit that referenced this pull request Dec 1, 2015
Merged by openshift-bot
@openshift-bot openshift-bot merged commit 96a1778 into openshift:master Dec 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants