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

Transition to Github Actions #26

Merged
merged 38 commits into from
Feb 23, 2021
Merged

Transition to Github Actions #26

merged 38 commits into from
Feb 23, 2021

Conversation

bernardopacini
Copy link
Collaborator

@bernardopacini bernardopacini commented Feb 13, 2021

Purpose

This change adds Github Actions and removes Travis testing. The Github Actions configuration file is in .github/workflows/. Also, this change pins the OpenMDAO version in requirements.txt to match the one tested in travis, the latest version fails the tests.

Type of change

This is a maintenance update.

Select the appropriate type(s) that describe this PR

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
    X Other (please describe)

Testing

Tests pass on personal fork, push to coveralls not tested yet (must come from MDOLab repo).

Checklist

Put an x in the boxes that apply.

  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@coveralls
Copy link

coveralls commented Feb 13, 2021

Pull Request Test Coverage Report for Build 570172305

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 87.491%

Files with Coverage Reduction New Missed Lines %
openconcept/utilities/visualization.py 2 15.38%
Totals Coverage Status
Change from base Build 178: -0.05%
Covered Lines: 3728
Relevant Lines: 4261

💛 - Coveralls

@bernardopacini
Copy link
Collaborator Author

Addresses issue #24.

@bernardopacini
Copy link
Collaborator Author

bernardopacini commented Feb 13, 2021

Testing with Github Actions is up and running besides the push to Coveralls. @bbrelje Do you remember how you set that up originally? Do we need to reset the repo on Coveralls for it to recognize Github Actions instead of Travis?

EDIT: Fixed (see below)

@bernardopacini
Copy link
Collaborator Author

Fixed coveralls push by switching to coveralls package instead of python-coveralls. The latter appears to be geared towards Travis specifically. This switch is also consistent with OpenAeroStruct which uses coveralls.

@bernardopacini bernardopacini marked this pull request as ready for review February 13, 2021 06:15
@bernardopacini bernardopacini requested a review from a team as a code owner February 13, 2021 06:15
@bernardopacini
Copy link
Collaborator Author

Before merging, Travis should be removed and Github Actions should be set as the required check. I do not believe I have access to make this change.

Also, AppVeyor is failing because it uses the wrong version of OpenMDAO (3.7 as opposed to 3.2.1). Do we want to fix this and continue using AppVeyor or remove it to rely on Github Actions?

@bernardopacini
Copy link
Collaborator Author

Fixed AppVeyor by pinning OpenMDAO version (3.2.1). Github Actions can test on Windows. Do we want to continue running AppVeyor or consolidate to Github Actions with two stages, one for linux and one for windows?

@ewu63
Copy link
Collaborator

ewu63 commented Feb 13, 2021

I would prefer to pin the version in setup.py rather than in requirements.txt. Also, not sure where environment.yml is used.

I'm also ambivalent regarding AppVeyor, though if we can do exactly the same in GA I guess that would be nice.

EDIT: I see that environment.yml is used for conda. I would prefer to unify everything into setup.py, and then we should be able to install pip from conda and use that to install all dependencies on Windows I think.

@bernardopacini
Copy link
Collaborator Author

bernardopacini commented Feb 15, 2021

@bbrelje I am thinking of moving all of the required packages (besides coverage-related ones) into the setup.py file so they are installed automatically. This would be: sphinx, sphinx_rtd_theme, and redbaron. Do you have any issue with this?

@ewu63
Copy link
Collaborator

ewu63 commented Feb 15, 2021

Any idea where the redbaron dependency comes from? I feel like I've seen that before

@kanekosh
Copy link
Contributor

Any idea where the redbaron dependency comes from? I feel like I've seen that before

I think redbaron is one of the OpenMDAO requirements to build the sphinx docs.

@ewu63
Copy link
Collaborator

ewu63 commented Feb 16, 2021

I think redbaron is one of the OpenMDAO requirements to build the sphinx docs.

Right thank you! We probably want to do openmdao[docs] or something then. I will try that

@ewu63
Copy link
Collaborator

ewu63 commented Feb 16, 2021

Sorry for the excessive commits, after a really long time I figured out that for miniconda to work we had to set bash differently. Everything is in place now, and I also incremented the version.

@ewu63
Copy link
Collaborator

ewu63 commented Feb 18, 2021

@bbrelje @kanekosh this PR is ready for you to review.

@ewu63 ewu63 merged commit 4705d7b into master Feb 23, 2021
@ewu63 ewu63 deleted the gha_transition branch February 23, 2021 15:41
@ewu63 ewu63 mentioned this pull request Feb 23, 2021
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.

None yet

5 participants