Skip to content

Attempt to migrate from Travis to GitHub Actions #302

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 29 commits into from
Mar 6, 2023

Conversation

peternewman
Copy link
Member

@peternewman peternewman commented Feb 17, 2023

First bit done for now...

peternewman and others added 22 commits July 24, 2022 13:30
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Also try the non-Git codespell
@peternewman peternewman marked this pull request as ready for review February 17, 2023 03:11
@peternewman peternewman requested a review from kripton February 17, 2023 03:11
peternewman and others added 6 commits February 21, 2023 16:29
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Please check this manually before merging
Manufacturer updates for GitHub Actions
@peternewman peternewman requested review from kripton and removed request for kripton March 5, 2023 22:00
@peternewman
Copy link
Member Author

This gets us back to at least having some basic CI again...

Copy link
Member

@kripton kripton left a comment

Choose a reason for hiding this comment

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

LGTM

strategy:
fail-fast: false
matrix:
task: [data-check, codespell]
Copy link
Member

Choose a reason for hiding this comment

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

Interesting use of matrix builds. I would've expected several steps/jobs instead of tasks defined by a matrix but why not?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was simply to get it the closest to our existing Travis setup, given it currently just calls the old script for that.

I think if you had steps/jobs, then where you've got say json-spec needed on more than one, you'd have to duplicate the install instructions (or install it even for the builds which don't need it, which doesn't seem very efficient to me.

if [ "$TASK" = "pychecker" -o "$TASK" = "pychecker-wip" ]; then pip install http://sourceforge.net/projects/pychecker/files/pychecker/0.8.19/pychecker-0.8.19.tar.gz/download; fi
#if [ "$TASK" = "codespell" ]; then pip install git+https://github.com/codespell-project/codespell.git; fi
if [ "$TASK" = "codespell" ]; then pip install codespell; fi
sudo apt-get install xvfb
Copy link
Member

Choose a reason for hiding this comment

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

Why not have those four apt install lines merged into one line with all packages installed in one go?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that's a fair point. I think I was running them individually as the presence of a package already being installed broke the whole build!

Copy link
Member Author

Choose a reason for hiding this comment

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

See also #304

@peternewman peternewman merged commit 8ef069e into master Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants