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

fix: add missing files for make bundle #167

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

Madhu-1
Copy link
Contributor

@Madhu-1 Madhu-1 commented Apr 21, 2022

After running make bundle locally few files got updated and new files also got added.

Signed-off-by: Madhu Rajanna [email protected]

/cc @nbalacha

@openshift-ci openshift-ci bot requested a review from nbalacha April 21, 2022 03:16
@Madhu-1 Madhu-1 changed the title fix: add missing file for make bundle fix: add missing files for make bundle Apr 21, 2022
Copy link
Contributor

@nbalacha nbalacha left a comment

Choose a reason for hiding this comment

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

Some of these do not need to be updated. They are temporary files generated from templates.
However, bundle/manifests/lvm-operator.clusterserviceversion.yaml needs to be updated as the image is incorrect.

@@ -1,5 +1,5 @@
# Adds namespace to all resources.
Copy link
Contributor

@nbalacha nbalacha Apr 21, 2022

Choose a reason for hiding this comment

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

This was updated by the make bundle target as we use different ns for upstream and downstream and ca be ignored.

@@ -285,7 +285,7 @@ spec:
valueFrom:
fieldRef:
fieldPath: metadata.name
image: quay.io/nibalach/lvm-operator:e2e
image: quay.io/ocs-dev/lvm-operator:latest
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should be committed.

@@ -0,0 +1,21 @@
apiVersion: apps/v1
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a file generated by make update-mgr-env and does not need to be committed.

@@ -3,3 +3,10 @@ resources:

Copy link
Contributor

Choose a reason for hiding this comment

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

Not required to be committed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nbalacha how about adding unwanted files to gitignore?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it is required. If it makes things less confusing, we can merge the temporary files as they will be overwritten anyway by the make targets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes as you mentioned it's less confusing and helps both contributors and maintainers to keep track of it.

After running `make bundle` locally few files
got updated and new files also got added.

Signed-off-by: Madhu Rajanna <[email protected]>
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 22, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Madhu-1, nbalacha

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 22, 2022
@openshift-merge-robot openshift-merge-robot merged commit 125e177 into openshift:main Apr 22, 2022
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants