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

refactor toxenvs, use OpenAstronomy workflows, and move MacOS jobs to schedule #75

Merged
merged 20 commits into from Mar 22, 2023

Conversation

zacharyburnett
Copy link
Collaborator

@zacharyburnett zacharyburnett commented Dec 15, 2022

This PR abstracts CI workflows to the OpenAstronomy workflows; this should reduce maintenance for updating and maintaining actions. Additionally, these changes move the majority of MacOS jobs to the weekly scheduled workflow. This should alleviate the issue where the limited MacOS runners for the organization are not available for CI jobs.

  • add tox.ini with toxenvs from Romancal
  • test various Python versions on ubuntu and macos
  • use ruff instead of flake8 to check lint rules moved to replace flake8 with ruff #77

image

after merging, the Required Statuses should also be updated

@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Patch and project coverage have no change.

Comparison is base (809607c) 54.17% compared to head (4c8d77b) 54.17%.

❗ Current head 4c8d77b differs from pull request most recent head 5959550. Consider uploading reports for the commit 5959550 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #75   +/-   ##
=======================================
  Coverage   54.17%   54.17%           
=======================================
  Files          25       25           
  Lines        3057     3057           
=======================================
  Hits         1656     1656           
  Misses       1401     1401           

see 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Collaborator

@stscieisenhamer stscieisenhamer left a comment

Choose a reason for hiding this comment

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

General curmudgeon/grey-beard start about flake8 to ruff: Why? Speed-up is a good thing, however version "0.0.187", though usually I do not take much stock in versions, still seems a bit early. Big question is IDE support: Is there any? Also, losing the flake8 configuration will re-introduce unneeded noise. Also, how will this play with depending packages?

Regardless, will need some developer documentation at least stating what is being used.

@zacharyburnett
Copy link
Collaborator Author

zacharyburnett commented Dec 19, 2022

General curmudgeon/grey-beard start about flake8 to ruff: Why? Speed-up is a good thing, however version "0.0.187", though usually I do not take much stock in versions, still seems a bit early. Big question is IDE support: Is there any? Also, losing the flake8 configuration will re-introduce unneeded noise. Also, how will this play with depending packages?

Regardless, will need some developer documentation at least stating what is being used.

Mainly, I am looking toward ruff for speedup, and support for pyproject.toml (to reduce clutter in the root directory). You're right however that it's still in development; I will separate the change into another PR so we can decide on it separately.

EDIT: moved to #77

Copy link
Collaborator

@stscieisenhamer stscieisenhamer left a comment

Choose a reason for hiding this comment

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

LGTM.

Related things that could probably be done, in other PRs, include updating the README to document how to install, setup development, test, etc. About time this becomes a proper package.

Also, for the group itself, a discussion on the changes/new stuff, such as tox and linting changes, would be in order. Possibly at an all-hands.

@WilliamJamieson
Copy link
Collaborator

@zacharyburnett it appears there is an issue with CRDS for jwst and romancal, can you take a look at these? I would like to merge this before merging #79.

tox.ini Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
@WilliamJamieson
Copy link
Collaborator

@zacharyburnett it looks like CRDS is working for JWST now but not for Roman. Is this because there is not a public CRDS for Roman yet?

@zacharyburnett zacharyburnett changed the title refactor CI refactor toxenvs, use OpenAstronomy workflows, and move MacOS jobs to schedule Mar 14, 2023
tox.ini Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
@zacharyburnett zacharyburnett removed the no-changelog-entry-needed does not require an entry in `CHANGES.rst` label Mar 22, 2023
@zacharyburnett zacharyburnett merged commit 970286c into spacetelescope:master Mar 22, 2023
16 checks passed
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

3 participants