Skip to content

ci: move scripts from '.github/gha' and '.github/travis' into '.github/scripts' #1804

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

Merged
merged 3 commits into from
Jul 30, 2021

Conversation

umarcor
Copy link
Contributor

@umarcor umarcor commented Jul 18, 2021

Description

This PR moves scripts from .github/gha and .github/travis into .github/scripts. Scripts that use those resources are updated accordingly. BTW, the workflow YAML is slightly simplified.

Related Issue

Close #1755

Motivation and Context

Travis is non used anymore.

How Has This Been Tested?

CI is still all green.

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Additional context

There are some remaining references to travis:

# grep -lr ./ -e "travis"
./abc/README.md
./libs/EXTERNAL/capnproto/README.md
./libs/EXTERNAL/libtatum/README.md

However, I assume that those are vendores dependencies, so they handle CI on their own.

@github-actions github-actions bot added the infra Project Infrastructure label Jul 18, 2021
@@ -1,5 +1,7 @@
#!/usr/bin/env bash

source $(dirname "$0")/../.github/scripts/common.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the goal was to keep these scripts from depending on stuff under the .github directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. That's what I thought. However, the common.sh is currently explicitly sourced. See

Format:
runs-on: ubuntu-18.04
strategy:
fail-fast: false
matrix:
include:
- { name: 'C/C++', script: 'check-format.sh' }
- { name: 'Python', script: 'check-format-py.sh' }
- { name: 'Python Lint', script: 'pylint_check.py' }
name: 'F: ${{ matrix.name }}'
steps:
- uses: actions/checkout@v2
- run: ./.github/gha/install_dependencies.sh
- uses: actions/setup-python@v2
with:
python-version: 3.6
- run: pip install -r requirements.txt
- name: Test
run: |
source .github/travis/common.sh
./dev/${{ matrix.script }}

I tried to do minimal functional modifications in this PR. So, precisely, I preserved common to be sourced before/in check-format.sh and check-formar-py.sh, but I did not in pylint_check.py.

If you or any maintainer/developer can confirm that common.sh is not required in check-format.sh/check-formar-py.sh, I will update them.

Moreover, the git blame belongs to me. So, this is inherited from the previous .travis.yml file. There, the common script was source in all the jobs: 13a8c9f#diff-6ac3f79fc25d95cd1e3d51da53a4b21b939437392578a35ae8cd6d5366ca5485L243-L254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Project Infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove travis name from CI scripts
4 participants