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

Generate checluster references tables #1803

Merged
merged 13 commits into from
Feb 15, 2021
Merged

Generate checluster references tables #1803

merged 13 commits into from
Feb 15, 2021

Conversation

tolusha
Copy link
Contributor

@tolusha tolusha commented Jan 21, 2021

Signed-off-by: Anatolii Bazko abazko@redhat.com

Read our Contribution guide before submitting a PR.

What does this PR do?

Add scripts to sync doc and checluster crd

What issues does this PR fix or reference?

https://issues.redhat.com/browse/RHDEVDOCS-2216

PR Checklist

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

  • vale has been run successfully against the PR branch
  • Link checker has been run successfully against the PR branch
  • Documentation describes a scenario that is already covered by QE tests, otherwise an issue has been created and acknowledged by Che QE team
  • Changed article references are updated where they are used (or a redirect has been set up on the docs side):

@tolusha tolusha requested review from l0rd and removed request for a team January 21, 2021 12:28
@MichalMaler MichalMaler self-assigned this Jan 21, 2021
@MichalMaler MichalMaler marked this pull request as draft January 21, 2021 13:15
@MichalMaler
Copy link
Contributor

LGTM. We are waiting for @l0rd approval now.

@tolusha tolusha marked this pull request as ready for review January 22, 2021 07:45
@tolusha
Copy link
Contributor Author

tolusha commented Jan 22, 2021

gen script requires yq installed since it will be run on Jenkins

@MichalMaler
Copy link
Contributor

gen script requires yq installed since it will be run on Jenkins

We can add:

.Prerequisites

@themr0c
Copy link
Contributor

themr0c commented Jan 22, 2021

The che-docs container has yq inside.

@themr0c
Copy link
Contributor

themr0c commented Jan 22, 2021

Can you confirm it runs on che.openshift.io?

@tolusha
Copy link
Contributor Author

tolusha commented Jan 22, 2021

I've just checked:

  1. Created a workspace from the devfile [1]
  2. Run yq command in che-docs containers.
    yes, it exists there.

[1] https://raw.githubusercontent.com/eclipse/che-docs/master/devfile.yaml

@themr0c
Copy link
Contributor

themr0c commented Jan 22, 2021

@MichalMaler the requirement is to build the docs, not to the user-facing docs.

Copy link
Contributor

@themr0c themr0c left a comment

Choose a reason for hiding this comment

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

Can you fix the shellcheck errors:

shellcheck tools/checluster_docs_gen.sh

In tools/checluster_docs_gen.sh line 17:
TABLE_HEADER="$NEWLINE[cols="2,5", options="header"]$NEWLINE:=== $NEWLINE Property: Description $NEWLINE"
^-- SC1087: Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
^----^ SC2140: Word is of the form "A"B"C" (B indicated). Did you mean "ABC" or "A"B"C"?

In tools/checluster_docs_gen.sh line 19:
PARENT_PATH=$( cd "$(dirname "${BASH_SOURCE[0]}")/.." ; pwd -P )
^-- SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

Did you mean:
PARENT_PATH=$( cd "$(dirname "${BASH_SOURCE[0]}")/.." || exit ; pwd -P )

In tools/checluster_docs_gen.sh line 27:
if [ $? -ne 0 ]; then
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.

In tools/checluster_docs_gen.sh line 41:
if [ $? -ne 0 ]; then
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.

In tools/checluster_docs_gen.sh line 59:
if [ $? -ne 0 ]; then
^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.

In tools/checluster_docs_gen.sh line 88:
local section=$(echo "$RAW_CONTENT" | yq -M '.spec.validation.openAPIV3Schema.properties.status')
^-----^ SC2155: Declare and assign separately to avoid masking return values.

In tools/checluster_docs_gen.sh line 90:
local section=$(echo "$RAW_CONTENT" | yq -M '.spec.validation.openAPIV3Schema.properties.spec.properties.'$sectionName)
^-----^ SC2155: Declare and assign separately to avoid masking return values.
^----------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
local section=$(echo "$RAW_CONTENT" | yq -M '.spec.validation.openAPIV3Schema.properties.spec.properties.'"$sectionName")

In tools/checluster_docs_gen.sh line 94:
$(echo "$section" | yq -M '.properties | keys[]' )
^-- SC2207: Prefer mapfile or read -a to split command output (or quote to avoid splitting).

In tools/checluster_docs_gen.sh line 103:
description=$(echo "$section" | yq -M '.properties.'$prop'.description')
^---^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
description=$(echo "$section" | yq -M '.properties.'"$prop"'.description')

For more information:
https://www.shellcheck.net/wiki/SC1087 -- Use braces when expanding arrays,...
https://www.shellcheck.net/wiki/SC2140 -- Word is of the form "A"B"C" (B in...
https://www.shellcheck.net/wiki/SC2155 -- Declare and assign separately to ...

@tolusha
Copy link
Contributor Author

tolusha commented Jan 22, 2021

@themr0c
Sorry, to be honest I don't think that they are relevant. Where did they come from ?

@l0rd
Copy link
Contributor

l0rd commented Jan 25, 2021

@themr0c
Sorry, to be honest I don't think that they are relevant. Where did they come from ?

@tolusha shellcheck is the most widespread linter for bash scripts (22K stars) and it's common to run our scripts against it.

@tolusha
Copy link
Contributor Author

tolusha commented Jan 26, 2021

@themr0c
Fixed

@MichalMaler
Copy link
Contributor

@themr0c Fabrice, I think we are good to go here :)
Good job @tolusha and thank you, @themr0c for setting up the linter.

@tolusha
Copy link
Contributor Author

tolusha commented Jan 26, 2021

$ shellcheck checluster_docs_gen.sh 
$ 

no issues with spellchecker anymore ...

@themr0c
Copy link
Contributor

themr0c commented Jan 26, 2021

So now, what can we do for the vale errors?

@MichalMaler
Copy link
Contributor

let me take a look

@themr0c
Copy link
Contributor

themr0c commented Jan 26, 2021

To fix vale errors:

For the static file, ref_checluster-custom-resource-fields-reference.adoc, this is straightforward: fix all vale errors in place.

For the generated file:

  • Long term: fix the upstream of the file.
  • Short term: substitutions, as we do with environment_docs_gen.sh

@tolusha
Copy link
Contributor Author

tolusha commented Jan 26, 2021

Too much of vale errors, that's not possible to fix at script using substitution.
I suggest the following:

  1. Fix build error and merge.
  2. Doc team reviews CRD file to fix descriptions there.
  3. Regenerate the doc with a new CRD

@tolusha
Copy link
Contributor Author

tolusha commented Feb 10, 2021

I've just updated PR based on the latest changes from master branch of che-operator to see if we have any vale errors.

@tolusha
Copy link
Contributor Author

tolusha commented Feb 12, 2021

I've update checluster doc based on master che-operator branch. So, we don't have value errors anymore.
But jenkins still fails.

@tolusha
Copy link
Contributor Author

tolusha commented Feb 12, 2021

NB: If PR is merged then checluster references tables will be regenerated based on 7.25 version of che-operator (from antora-playbook.yaml) which doesn't contain all vale fixes.

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@tolusha tolusha marked this pull request as ready for review February 12, 2021 12:11
@tolusha tolusha merged commit 0b23939 into eclipse-che:master Feb 15, 2021
@tolusha tolusha deleted the checluster branch February 15, 2021 10:44
MichalMaler added a commit that referenced this pull request Feb 15, 2021
* Generate checluster references tables

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Co-authored-by: Michal Maléř <48474054+MichalMaler@users.noreply.github.com>
@che-bot che-bot added this to the 7.27 milestone Feb 15, 2021
@themr0c themr0c modified the milestones: 7.27, 7.26.x Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants