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

Property descriptions #19233

Merged
merged 5 commits into from
Mar 11, 2021
Merged

Conversation

xbaran4
Copy link
Contributor

@xbaran4 xbaran4 commented Mar 9, 2021

Signed-off-by: xbaran4 pbaran@redhat.com

What does this PR do?

Adds description to properties missing it.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

#18572

How to test this PR?

Build and deploy on minikube with chectl.

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

Signed-off-by: xbaran4 <pbaran@redhat.com>
Signed-off-by: xbaran4 <pbaran@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Mar 9, 2021

Can one of the admins verify this patch?

@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Mar 9, 2021
@sparkoo sparkoo requested review from metlos and sparkoo March 9, 2021 09:47
Copy link
Member

@sparkoo sparkoo left a comment

Choose a reason for hiding this comment

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

nice work. Please see my comments

che.oauth.github.clientid=NULL

# Your GitHub OAuth client secret.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Your GitHub OAuth client secret.
# GitHub OAuth client secret.

# You can setup GitHub OAuth to automate authentication to remote repositories.
# You need to first register this application with GitHub OAuth.
# Your GitHub OAuth client ID.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Your GitHub OAuth client ID.
# GitHub OAuth client ID.

che.infra.kubernetes.master_url=

# Configuration of Kubernetes client `whether to use trusted certificates` that Infra will use.
Copy link
Member

Choose a reason for hiding this comment

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

This description feels a bit clumsy. Why "whether to use trusted certificates
" is in a quotes?
What is the value type?

I see it's boolean, so without any value here it defaults to false? Maybe we should write false here explicitly.

# Data for TLS Secret that should be used for workspaces Ingresses
# cert and key should be encoded with Base64 algorithm
# These properties are ignored by OpenShift infrastructure
# Key for TLS Secret that should be used for workspaces Ingresses
Copy link
Member

Choose a reason for hiding this comment

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

please write explicitly here that it is key data

che.infra.kubernetes.tls_key=NULL

# Certificate for TLS Secret that should be used for workspaces Ingresses
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Certificate for TLS Secret that should be used for workspaces Ingresses
# Certificate for TLS Secret that should be used for workspaces Ingresses.

# plugins dependencies to a workspace
#
# Note these images are overridden by the Che Operator by default; changing the images here will not
# Docker image of plugin broker metadata.
Copy link
Member

Choose a reason for hiding this comment

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

I think "plugin metadata broker" would fit better

# have an effect if Che is installed via Operator.
che.workspace.plugin_broker.metadata.image=quay.io/eclipse/che-plugin-metadata-broker:v3.4.0

# Docker image of Che plugin broker artifacts.
Copy link
Member

Choose a reason for hiding this comment

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

"plugin artifacts broker"

# This broker runs as an init container on the workspace pod. Its job is to take in a list of plugin identifiers
# (either references to a plugin in the registry or a link to a plugin meta.yaml) and ensure that the correct .vsix
# and .theia extenions are downloaded into the /plugins directory, for each plugin requested for the workspace.
# Note this image is overridden by the Che Operator by default; changing the image here will not
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to write here "changing the image here will not have an effect if Che is installed via Operator.". It's implicit for all values set externally with env variables.

# which doesn't contain any Che-specific workspace descriptor.
che.factory.default_editor=eclipse/che-theia/next

# Plugins t will be used for factories which are created from remote git repository
Copy link
Member

Choose a reason for hiding this comment

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

"t" ? :)

Signed-off-by: xbaran4 <pbaran@redhat.com>
Signed-off-by: xbaran4 <pbaran@redhat.com>
@xbaran4 xbaran4 closed this Mar 10, 2021
@xbaran4 xbaran4 deleted the property_descriptions branch March 10, 2021 09:45
@xbaran4 xbaran4 restored the property_descriptions branch March 10, 2021 09:46
@xbaran4 xbaran4 reopened this Mar 10, 2021
@xbaran4 xbaran4 mentioned this pull request Mar 10, 2021
9 tasks
@sparkoo sparkoo merged commit 69fa3bf into eclipse-che:master Mar 11, 2021
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Mar 11, 2021
@che-bot che-bot added this to the 7.28 milestone Mar 11, 2021
@xbaran4 xbaran4 deleted the property_descriptions branch May 19, 2021 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Outline of a bug - must adhere to the bug report template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants