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

Some small types fixed in readme and makefile #423

Merged
merged 1 commit into from
Jul 11, 2022
Merged

Conversation

jdmaguire
Copy link
Contributor

@jdmaguire jdmaguire commented Jul 10, 2022

Motivation

I was following the README and found two small issues preventing me from running the code.

Modifications

One was a typo (I think) for the operator-sdk. The README has --controller-true. I think this is a mistake since I see the operation is --controller=true in the operator-sdk docs and cli on my computer.

The second issue I had was when running make run. Main.go would crash setting up the webhook. Reading

function-mesh/main.go

Lines 151 to 153 in 4ed298a

// enable the webhook service by default
// Disable function-mesh webhook with `ENABLE_WEBHOOKS=false` when we run locally.
if os.Getenv("ENABLE_WEBHOOKS") != "false" {
, it seems ENABLE_WEBHOOKS should be set to false when running locally with make run. I've set it to thus. (An alternative 'fix' would be the README saying to run ENABLE_WEBHOOKS=false make run as opposed to make run.)

These may both be non-issues. Feel free to correct any misunderstanding I have.

Thanks!

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: no

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    (Your PR needs to update docs and you will update later)

  • [ x ] doc-not-needed
    It is cleaning up docs

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@jdmaguire jdmaguire requested review from nlu90, freeznet and a team as code owners July 10, 2022 21:39
@github-actions
Copy link

@jdmaguire:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@github-actions github-actions bot added the doc-info-missing This pr needs to mark a document option in description label Jul 10, 2022
@jdmaguire jdmaguire force-pushed the fixup-docs-and-make branch from d2e82ef to 4691526 Compare July 10, 2022 21:45
@freeznet freeznet added type/bug doc This pr contains a document m/2022-07 labels Jul 11, 2022
@github-actions
Copy link

@jdmaguire:Thanks for providing doc info!

@github-actions github-actions bot removed the doc-info-missing This pr needs to mark a document option in description label Jul 11, 2022
@freeznet freeznet merged commit 8e55f8e into master Jul 11, 2022
@freeznet freeznet deleted the fixup-docs-and-make branch July 11, 2022 03:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc This pr contains a document m/2022-07 type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants