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.

Sign up for GitHub

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

Jump to bottom

templates: cleanup: rename all *.service templates to *.service.yaml #1926

Merged
merged 1 commit into from Jul 22, 2020
Merged

templates: cleanup: rename all *.service templates to *.service.yaml #1926

merged 1 commit into from Jul 22, 2020

Conversation

LorbusChris
Copy link
Member

@LorbusChris LorbusChris commented Jul 14, 2020 •
edited

All templates are yaml files, and this should be reflected in their file name suffixes.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 14, 2020
@ericavonb
Copy link
Contributor

Should this PR have a WIP on it? Not sure what's going on or what the context is here.

@yuqi-zhang
Copy link
Contributor

All our template files are written in yaml but we're indifferent to the actual naming scheme when parsing them. I think this is just a minor cleanup to make sure they are named with a .yaml suffix (As opposed to a .service suffix since they're not "service files"). Shouldnt error either way

@LorbusChris LorbusChris changed the title templates: Make all templates into yaml files templates: cleanup: Make all templates into yaml files Jul 14, 2020
@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Jul 15, 2020 •
edited

All our template files are written in yaml but we're indifferent to the actual naming scheme when parsing them. I think this is just a minor cleanup to make sure they are named with a .yaml suffix (As opposed to a .service suffix since they're not "service files"). Shouldnt error either way

If this is indeed the case, @LorbusChris can you please add this rationale to the commit message just so it's clear? Or maybe like " templates: cleanup: rename all *.service templates to *.service.yaml " or something?

@LorbusChris LorbusChris mentioned this pull request Jul 16, 2020
Closed
@ericavonb
Copy link
Contributor

Got it. I don't see any problems with the change. @LorbusChris ping when you've updated the commit message and I can add the lgtm

@kikisdeliveryservice kikisdeliveryservice changed the title templates: cleanup: Make all templates into yaml files [WIP] templates: cleanup: Make all templates into yaml files Jul 21, 2020
@kikisdeliveryservice kikisdeliveryservice added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 21, 2020
@kikisdeliveryservice
Copy link
Contributor

Just threw a WIP on this since it needs a rebase/update commit as requested. Take it off when ready 😺

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2020
@LorbusChris LorbusChris changed the title [WIP] templates: cleanup: Make all templates into yaml files templates: cleanup: rename all *.service templates to *.service.yaml Jul 22, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 22, 2020
@LorbusChris
templates: cleanup: rename all *.service templates to *.service.yaml
79fb531 
All templates are yaml files, and this should be reflected in their file name suffixes.
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2020
@LorbusChris
Copy link
Member Author

LorbusChris commented Jul 22, 2020 •
edited

@kikisdeliveryservice retitled per your comment, should be good to go :)

/retest

@kikisdeliveryservice
Copy link
Contributor

Thanks @LorbusChris

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 22, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kikisdeliveryservice, LorbusChris, yuqi-zhang

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [kikisdeliveryservice,yuqi-zhang]

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

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jul 22, 2020 •
edited

@LorbusChris: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-proxy fec9850fb67a3dc22506938dd4ee409959202408 link /test e2e-aws-proxy

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 422cbf4 into openshift:master Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuqi-zhang yuqi-zhang yuqi-zhang approved these changes

@ericavonb ericavonb Awaiting requested review from ericavonb

Assignees

@kikisdeliveryservice kikisdeliveryservice

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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@LorbusChris @ericavonb @yuqi-zhang @kikisdeliveryservice @openshift-ci-robot @openshift-bot @openshift-merge-robot