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

Adding code of conduct and contribute guide #30

Merged
merged 3 commits into from
Nov 11, 2022

Conversation

Skarlso
Copy link
Contributor

@Skarlso Skarlso commented Nov 11, 2022

No description provided.

Copy link
Contributor

@phoban01 phoban01 left a comment

Choose a reason for hiding this comment

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

LGTM. I would change the suggested port for the kind registry to 5000 as the ocm-controller starts a registry by default on port 5001 🛩️

@Skarlso
Copy link
Contributor Author

Skarlso commented Nov 11, 2022

Oh nice find. :D

@Skarlso
Copy link
Contributor Author

Skarlso commented Nov 11, 2022

The only problem is that the kind guide uses 5001. So which one to we adjust then... 🤔 I think it makes more sense for the controller to be adjusted?

Something like:

./bin/manager -zap-log-level 4 --oci-registry-port 5000

@phoban01
Copy link
Contributor

The only problem is that the kind guide uses 5001. So which one to we adjust then... thinking I think it makes more sense for the controller to be adjusted?

Something like:

./bin/manager -zap-log-level 4 --oci-registry-port 5000

I agree. Should we adjust the default too?

@Skarlso
Copy link
Contributor Author

Skarlso commented Nov 11, 2022

I agree. Should we adjust the default too?

Humm hummmmm... Sure. We have to do that in the other controllers as well then. :)

@Skarlso
Copy link
Contributor Author

Skarlso commented Nov 11, 2022

Hahaha, check this out. The replication controller already defaults to 5000... :D

	flag.StringVar(&ociRegistryAddr, "oci-registry-addr", "localhost:5000", "The address to the OCI registry.")

@phoban01
Copy link
Contributor

That address should really point at Kubernetes service name for the deployment but be overridable for local testing, similar to flux kustomization and source controller.

@Skarlso
Copy link
Contributor Author

Skarlso commented Nov 11, 2022

That address should really point at Kubernetes service name for the deployment but be overridable for local testing, similar to flux kustomization and source controller.

You usually do that in the deployment, right? To point to the right service name and DNS record...

Copy link
Contributor

@phoban01 phoban01 left a comment

Choose a reason for hiding this comment

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

LGTM

@Skarlso Skarlso merged commit ec242fa into main Nov 11, 2022
@phoban01 phoban01 deleted the adding_contrib_and_code_of_conduct branch December 21, 2022 09:58
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.

2 participants