-
Notifications
You must be signed in to change notification settings - Fork 101
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
validate release actions support mulit os #197
validate release actions support mulit os #197
Conversation
COUNT=$(find . -type f -size +900k | wc -l) | ||
if [[ $COUNT -ne 0 ]]; then | ||
find . -type f -size +900k | ||
echo "The package shouldn't include file larger than 900kb, but get $COUNT" | ||
echo "The package shouldn't include file larger than 900kb, but get $COUNT" && exit 1 | ||
fi | ||
COUNT=$(find . -type f | perl -lne 'print if -B' | grep -v *.txt | wc -l) |
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.
need a better way to search binary files(and exclude some files marked in properties)
no we can't exit here,but we should search the key word in action log manually
@@ -250,3 +270,4 @@ jobs: | |||
fail-fast: false | |||
matrix: | |||
java_version: [ '8','11' ] | |||
os: [ubuntu-latest, macos-latest] # TODO: support windows-latest or other os in future |
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.
perfer not to use comment after the line(use a separate line)
and seems miss centos here?
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 try to find centos, It doesn't seem to be found
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.
seems github does not support CentOS, the supported runners: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources
I think we can mark "TODO: support CentOS" now.
how to support CentOS: https://zhuanlan.zhihu.com/p/344367359
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.
can we move validate-release.sh from hugegraph-server to hugegraph-doc?
@@ -180,6 +182,7 @@ jobs: | |||
echo "test hubble" | |||
cd ./*hubble*${{ inputs.release_version }} || exit | |||
cat conf/hugegraph-hubble.properties && bin/start-hubble.sh | |||
# TODO: need stop the server here |
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 can also add # kill the HugeGraphServer process by jps
at line 154
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 can also add
# kill the HugeGraphServer process by jps
at line 154
seems should stop server after toolchain test
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.
fi | ||
COUNT=$(find . -type f | perl -lne 'print if -B' | grep -v *.txt | wc -l) | ||
if [[ $COUNT -ne 0 ]]; then | ||
find . -type f | perl -lne 'print if -B' | ||
# due to the search script is not perfect, we can't exit here (check manually) | ||
echo "The package shouldn't include binary file, but get $COUNT" |
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.
Find a critical problem here, we need exclude the files with a white-list
& exit here:
*.txt
should be allowed, but whygrep -v *.txt
doesn't work?- exclude the file
/hugegraph-hubble/hubble-fe/src/assets/imgs/logo.png
- exclude the file
./hugegraph-hubble/hubble-fe/public/favicon.ico
- exclude the file
yarn.lock
This is the 1st priority we need to fix and enhance it
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.
Find a critical problem here, we need exclude the files with a
white-list
& exit here:
*.txt
should be allowed, but whygrep -v *.txt
doesn't work?- exclude the file
/hugegraph-hubble/hubble-fe/src/assets/imgs/logo.png
- exclude the file
./hugegraph-hubble/hubble-fe/public/favicon.ico
- exclude the file
yarn.lock
This is the 1st priority we need to fix and enhance it
1.should exit while check failed ?
2.use 'grep --exclude=*.{txt}' to exclude *.txt , and similar to others ?
exclude works in my repo test,see: https://github.com/z7658329/incubator-hugegraph-doc/actions/runs/4168898449/jobs/7216220550
@@ -212,8 +236,6 @@ jobs: | |||
cd dist/${{ inputs.release_version }} | |||
cd ./*hugegraph-incubating*${{ inputs.release_version }} || exit | |||
bin/init-store.sh && sleep 1 | |||
# kill the HugeGraphServer process by jps | |||
jps | grep HugeGraphServer | awk '{print $1}' | xargs kill -9 |
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 must stop the old one, otherwise can't start the new one
@@ -245,8 +267,12 @@ jobs: | |||
cat conf/hugegraph-hubble.properties | |||
bin/stop-hubble.sh && bin/start-hubble.sh | |||
cd - || exit | |||
# TODO: need stop the server here | |||
jps | grep HugeGraphServer | awk '{print $1}' | xargs kill -9 |
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.
sleep a while before the stop, like 60s?
@@ -180,6 +182,7 @@ jobs: | |||
echo "test hubble" | |||
cd ./*hubble*${{ inputs.release_version }} || exit | |||
cat conf/hugegraph-hubble.properties && bin/start-hubble.sh | |||
# TODO: need stop the server here |
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.
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 some comments to be addressed: https://github.com/apache/incubator-hugegraph-doc/pull/197/files
Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label |
@z7658329 merge this PR first, and enhance it later (due to we lack the script now 😢) Address the comment in another PR |
* add validate-release.sh --------- Co-authored-by: imbajin <jin@apache.org> 1167b17
* add validate-release.sh --------- Co-authored-by: imbajin <jin@apache.org> 1167b17
@imbajin could you please check all the comments are addressed? |
validate release actions support mulit os, test see:
https://github.com/z7658329/incubator-hugegraph-doc/actions/runs/4165118148/jobs/7207673497