-
Notifications
You must be signed in to change notification settings - Fork 56
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
build content docs - in gitlab #1619
base: master
Are you sure you want to change the base?
Conversation
cf713a1
to
487a091
Compare
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
2 similar comments
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you'd like to keep it open, please leave a comment with the status of the PR. Thank you for your contribution! |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
4c27803
to
10a66fd
Compare
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
4 similar comments
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
2 similar comments
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
Preview Site AvailableCongratulations! The automatic build has completed successfully. Important: Make sure to inspect your changes at the preview site. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
Approved, with comments.
index.json | ||
/.firebase/hosting.YnVpbGQ.cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this YnVpbGQ
magic value?
#!/bin/bash | ||
|
||
CYAN="\e[0;36m" | ||
CLEAR="\e[0m" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would put CLEAR
at the start or the end of this list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't be in the end since we use it in line 5
echo " 1. section_start \"<section-start-id>\" \"<section-header>\"" | ||
echo " 2. section_start \"<section-header>\"" | ||
echo " 3. section_start \"<section-start-id>\" \"<section-header>\" --collapse" | ||
echo " 4. section_start \"<section-header>\" --collapse" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to group by functionality
echo " 1. section_start \"<section-start-id>\" \"<section-header>\"" | |
echo " 2. section_start \"<section-header>\"" | |
echo " 3. section_start \"<section-start-id>\" \"<section-header>\" --collapse" | |
echo " 4. section_start \"<section-header>\" --collapse" | |
echo " 2. section_start \"<section-header>\"" | |
echo " 4. section_start \"<section-header>\" --collapse" | |
echo " 1. section_start \"<section-start-id>\" \"<section-header>\"" | |
echo " 3. section_start \"<section-start-id>\" \"<section-header>\" --collapse" |
start_time=$(date +%s) | ||
start="$(echo "$start" | sed -e "s/the_time/$start_time/" -e "s/section_id/$section_id/" -e "s/section_header/$section_header/")" | ||
echo -e "$start" | ||
date +"[%Y-%m-%dT%H:%M:%S.%3N] section start" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[%Y-%m-%dT%H:%M:%S.%3N]
is candidate for a CONST?
Used more than once...
section_header="$2" | ||
section_id="$1" | ||
else | ||
echo "section_start should be called with 1-3 args but it was called with $#" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
echo "section_start should be called with 1-3 args but it was called with $#" | |
echo "section_start should be called with 1-3 args but it was called with $# args" |
#echo "Installing pipenv..." | ||
#pipenv install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still needed?
echo "Generating docs..." | ||
pipenv run ./gendocs.py -t "${TARGET_DIR}" -d "${CONTENT_GIT_DIR}" -b "${CURRENT_BRANCH}" | ||
#pipenv run ./gendocs.py -t "${TARGET_DIR}" -d "${CONTENT_GIT_DIR}" -b "${CURRENT_BRANCH}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still needed?
echo "Generating Demisto class and CommonServerPython docs..." | ||
pipenv run ./gen_pydocs.py -t "${TARGET_DIR}" | ||
#pipenv run ./gen_pydocs.py -t "${TARGET_DIR}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still needed?
if [[ "$CURRENT_BRANCH" != "master" && "$CURRENT_BRANCH" != *"gen-top-contrib"* ]]; then | ||
echo "Skipping top contributors page generation, should run only on master or branch containing 'gen-top-contrib'." | ||
exit 0 | ||
else | ||
echo "Generating top contributors page..." | ||
pipenv run python ./gen_top_contrib.py -t "${CONTRIB_TARGET_DIR}" | ||
poetry run ./gen_top_contrib.py -t "${CONTRIB_TARGET_DIR}" | ||
# pipenv run python ./gen_top_contrib.py -t "${CONTRIB_TARGET_DIR}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still needed?
(In previous places it was before the poetry
:-)
@@ -135,7 +135,7 @@ def main(): | |||
PR_NUM: if set will use this as the pull request number. Otherwise will move on to CIRCLE_PULL_REQUEST | |||
CIRCLE_PULL_REQUEST: pull request url to use to get the pull id. Such as: https://github.com/demisto/content-docs/pull/9 | |||
if CIRCLE_PULL_REQUEST will try to get issue id from last commit comment (case of merge into master) | |||
CIRCLE_BRANCH: if set to master treats as a production deployment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CIRCLE_BRANCH
still appears in the code above (e.g. line 109)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!!
Approved with comments
- id: ruff | ||
args: | ||
- --fix | ||
exclude: CommonServerPython.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may want to exclude demistomock as well
@@ -0,0 +1,8 @@ | |||
variables: | |||
CURRENT_BRANCH_NAME: infra-content-docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be master?
|
||
include: | ||
- file: "/.gitlab/ci/content-docs/.gitlab-ci.yml" | ||
ref: infra-content-docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same ☝️
#!/bin/bash | ||
|
||
CYAN="\e[0;36m" | ||
CLEAR="\e[0m" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't be in the end since we use it in line 5
Status
In Progress
Related Issues
fixes: link to the issue
Description
gitlab CI build for docs