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 to GitHub Actions #675

Merged
merged 6 commits into from Feb 1, 2021
Merged

Migrate to GitHub Actions #675

merged 6 commits into from Feb 1, 2021

Conversation

weaverba137
Copy link
Member

This PR migrates CI testing to GitHub Actions. As a by-product, many documentation warnings were cleaned up.

@geordie666, the structure of desitarget looks a bit "shaggy" to me, for lack of a better word. Are you fully tracking the development of the package?

@weaverba137 weaverba137 self-assigned this Feb 1, 2021
@geordie666
Copy link
Contributor

Thanks @weaverba137. I think this looks good, and the proof is really that the tests pass with GitHub actions. Feel free to merge when ready.

In terms of whether I'm fully tracking development of desitarget, the answer is yes, except for:

  1. Anything in desitarget.mocks, for which I've never really done any development.
  2. Anything in desitarget.train, which is where we hold external code for random forests that can't be run directly in the DESI environment.
  3. Anything that is to do with end-to-end integration tests or the DESI environment, for which I trust Stephen.

Eventually, I'll get around to cleaning up 2.. But, for now, desitarget.train is just retained so we have an example of the code, rather than what was done for the RF vanishing completely.

I've also generally trusted sphinx and code style tests in the past, if they pass, although I appreciate that they don't always catch everything.

If anything else looks "shaggy," or if you have any pointers or criticisms, please reach out to me and I'll make sure to better maintain those elements of desitarget.

@weaverba137
Copy link
Member Author

@geordie666, thank you for the update. My main concern was that the doc/api.rst file had gotten very out of sync with the structure of the package. You might want to insist people update that, just like one would normally insist on updating doc/changes.rst.

@weaverba137 weaverba137 merged commit 187940a into master Feb 1, 2021
@weaverba137 weaverba137 deleted the github-actions branch February 1, 2021 23:57
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

2 participants