Skip to content
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

Migrate CI from azure pipelines to GitHub Actions #4730

Merged
merged 61 commits into from
Jan 11, 2021

Conversation

andersy005
Copy link
Member

@andersy005 andersy005 commented Dec 23, 2020

As discussed in today's dev meeting, it'd be convenient to migrate all our CIs from azure pipelines to GitHub Actions.

  • Consolidate all style/type checks (isort, black, mypy, flake8) into a single workflow and use the pre-commit GitHub action.
  • Create a main CI workflow and parametrize it for different settings
  • Add minimum version policy workflow
  • Add Doctests workflow
  • Remove azure pipelines config
  • Update docs: remove references to azure pipelines

@keewis
Copy link
Collaborator

keewis commented Dec 23, 2020

I think the EOF issue is a bug in blackdoc, it shouldn't remove the newline immediately before EOF.

To fix the mypy errors I would change the hook configuration to exclude properties and asv_bench:

  - repo: https://github.com/pre-commit/mirrors-mypy
    rev: v0.790  # Must match ci/requirements/*.yml
    hooks:
      - id: mypy
        exclude: "properties|asv_bench"

if we do want to type check those we could add a empty __init__.py to properties and fix the errors in asv_bench

@andersy005 andersy005 added the CI Continuous Integration tools label Dec 24, 2020
@andersy005 andersy005 marked this pull request as ready for review December 24, 2020 04:49
Copy link
Collaborator

@keewis keewis left a comment

Choose a reason for hiding this comment

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

thanks, @andersy005, this looks great.

The attendance of that meeting was pretty low so we should probably wait on a few more reviews.

.github/workflows/ci.yaml Show resolved Hide resolved
ci/requirements/environment.yml Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/ci-pre-commit.yml Show resolved Hide resolved
@jhamman
Copy link
Member

jhamman commented Dec 29, 2020

Thanks @andersy005 for pulling this together. This will make for a much cleaner CI experience.

@jhamman
Copy link
Member

jhamman commented Jan 7, 2021

@andersy005 - I merged #4688, so we just have some conflicts to sort out now.

@andersy005
Copy link
Member Author

@andersy005 - I merged #4688, so we just have some conflicts to sort out now.

I'm working on it. Will update this branch shortly

@andersy005 andersy005 requested review from keewis and jhamman January 7, 2021 18:45
@andersy005
Copy link
Member Author

Heres' what we get with [skip-ci] tag in the git commit message: Cc @dcherian

Screen Shot 2021-01-07 at 11 51 26 AM

Copy link
Collaborator

@keewis keewis left a comment

Choose a reason for hiding this comment

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

one question, otherwise this looks good to me.

python xarray/util/print_versions.py
- name: Import xarray
run: |
python -OO -c "import xarray"
Copy link
Collaborator

Choose a reason for hiding this comment

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

we currently don't use the -OO flag in the upstream-dev CI. Should we add it to the upstream-dev CI or remove it here and from CI Additional?

Copy link
Member

Choose a reason for hiding this comment

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

@andersy005 - thoughts on this one? Otherwise I think we merge this and follow up with dev doc updates.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have a preference. I used the -OO flag because it was present in the previous azure pipelines CI. For consistency, I'm going to remove it

Copy link
Collaborator

Choose a reason for hiding this comment

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

great, thanks. If we had a specific reason for -OO we can always add it back in a new PR, I guess.

@keewis
Copy link
Collaborator

keewis commented Jan 7, 2021

actually, we have a picture in the contributing docs which is really out-of-date now (there's still stickler and appveyor). Should we update that here, or is it fine to keep it until we rewrite the contributing docs (see #4361)?

@dcherian
Copy link
Contributor

dcherian commented Jan 7, 2021

Looks great. Thanks! I think we can wait to update the docs.

@keewis
Copy link
Collaborator

keewis commented Jan 11, 2021

Merging. If any issues remain we can fix them in new PRs.

@keewis keewis merged commit d241aa4 into pydata:master Jan 11, 2021
@andersy005 andersy005 deleted the ci/migrate-to-GHA branch January 11, 2021 12:24
dcherian added a commit to TomNicholas/xarray that referenced this pull request Jan 18, 2021
* upstream/master: (342 commits)
  fix decode for scale/ offset list (pydata#4802)
  Expand user dir paths (~) in open_mfdataset and to_zarr. (pydata#4795)
  add a version info step to the upstream-dev CI (pydata#4815)
  fix the ci trigger action (pydata#4805)
  scatter plot by order of the first appearance of hue (pydata#4723)
  don't skip the scheduled CI (pydata#4806)
  coords: retain str dtype (pydata#4759)
  Fix interval labels with units (pydata#4794)
  Always force dask arrays to float in missing.interp_func (pydata#4771)
  Print number of variables in repr (pydata#4762)
  install conda as a library in the minimum dependency check CI (pydata#4792)
  Migrate CI from azure pipelines to GitHub Actions (pydata#4730)
  use conda.api instead of parallel calls to the conda binary (pydata#4775)
  Speed up missing._get_interpolator (pydata#4776)
  Remove special case in guess_engines (pydata#4777)
  improve typing of OrderedSet (pydata#4774)
  CI: ignore some warnings (pydata#4773)
  DOC: update hyperlink for xskillscore (pydata#4778)
  drop support for python 3.6 (pydata#4720)
  Trigger upstream CI on cron schedule (by default) (pydata#4729)
  ...
@max-sixty
Copy link
Collaborator

This seems to be going well, thanks again!

The "Overriding CI behaviors" is fairly prominent in the PR template, when I wouldn't expect it to be used that often. Was that intended? Should it be a markdown comment?

@andersy005
Copy link
Member Author

The "Overriding CI behaviors" is fairly prominent in the PR template, when I wouldn't expect it to be used that often. Was that intended? Should it be a markdown comment?

The current choice was just a first stab at documenting the CI "special" behaviors.

👍🏽 to making the "overriding CI behaviors" section a markdown comment and documenting this in the contributing guide (for convenience) if need be.

triggered: ${{ steps.detect-trigger.outputs.trigger-found }}
steps:
- uses: actions/checkout@v2
- uses: ./.github/actions/detect-ci-trigger
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 action was moved into its own repository.

You can replace uses: ./.github/actions/detect-ci-trigger with

uses: xarray-contrib/ci-trigger@v1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants