-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[ci] Split the ci/scripts directory into ci/scripts/github and ci/scripts/jenkins #12941
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
Conversation
areusch
left a comment
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.
thanks @driazati , left a couple comments
ci/jenkins/macros.j2
Outdated
| script: """ | ||
| set -eux | ||
| . ci/scripts/retry.sh | ||
| . ci/scripts/jenkins/retry.sh |
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.
shall we make the jenkins script dir a var?
| from pathlib import Path | ||
| from typing import Dict, Tuple, Any, Optional, List, Union | ||
|
|
||
| # Hackery to enable importing of utils from ci/scripts/jenkins |
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 think at some point we were going to make this a Python package, right? is the idea that this hack bridges the gap til that happens?
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.
Yes, I think that's needed until we create the proper package structure.
|
|
||
|
|
||
| REPO_ROOT = Path(__file__).resolve().parent.parent.parent | ||
| REPO_ROOT = Path(__file__).resolve().parent.parent.parent.parent |
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.
could also
assert (REPO_ROOT / "Jenkinsfile").exists
for stability
tests/python/ci/test_ci.py
Outdated
| Test that reviewers are added from 'cc @someone' messages in PRs | ||
| """ | ||
| reviewers_script = REPO_ROOT / "ci" / "scripts" / "github_cc_reviewers.py" | ||
| reviewers_script = REPO_ROOT / "ci" / "scripts" / "github" / "github_cc_reviewers.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.
do we think this pattern is common enough to make GITHUB_SCRIPT_ROOT and JENKINS_SCRIPT_ROOT?
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.
a4b0489 to
923dcb2
Compare
…13368) This PR is a duplicate of apache#12940 and apache#12941. For some reason, I am unable to reopen apache#12940.
This is a copy of #12940 to get it running in CI. The original description is copied below:
This PR separates the files in the
ci/scriptsdirectory intoci/scripts/jenkinsandci/scripts/github. This is done because thegithubscripts should not be protected in the CI. #12604