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

ENH: workflows update and new docker files #448

Merged
merged 11 commits into from
Nov 15, 2023

Conversation

Gui-FernandesBR
Copy link
Member

@Gui-FernandesBR Gui-FernandesBR commented Oct 29, 2023

Pull request type

  • Code changes (bugfix, features)
  • Code maintenance (refactoring, formatting, tests)
  • ReadMe, Docs and GitHub updates
  • Other (please describe):

Checklist

  • Tests for the changes have been added (if needed)
  • Docs have been reviewed and added / updated
  • Lint (black rocketpy/ tests/) has passed locally
  • All tests (pytest --runslow) have passed locally

Current behavior

  • Our github actions only support python 3.11, but we should be already testing the new python 3.12.
  • It is hard to test if rocketpy is running in all the 3 supported OS before submitting a PR.

New behavior

  • Docker is here! I added a Dockerfile and a docker-compose.yml file to build and run rocketpy in Docker containers.
  • Some short instructions on how to use Docker were documented.
  • GitHub actions are still the most reliable and easy way to test rocketpy on different OS, but docker allows us to do it out of GitHub now, free of charge.
  • Travis CI was disabled, and its file deleted. We cannot use that workflow due to the free plan constrain. Also, it started to be a bit redundant since the last improvement of GitHub Actions.

Breaking change

  • No

Additional information

  • More complex codes can be done with Docker in this project, but I think this PR is already a good start.
  • RocketPy is building on Python 3.12, but the optional package timezonefinder can not be installed yet (see Python 3.12 compatibility jannikmi/timezonefinder#208)
  • TLDR: if you code on windows, test your code on linux using docker! (vice-versa)

@Gui-FernandesBR Gui-FernandesBR added the C.I. Continuous Integration (Workflows and actions) label Oct 29, 2023
@Gui-FernandesBR Gui-FernandesBR self-assigned this Oct 29, 2023
@Gui-FernandesBR
Copy link
Member Author

it would be nice if we fix (or simply delete) the action that makes actions fail if the PR is marked as draft.
It is not working, and sometimes it runs for hours because it never get closed. See an example: https://github.com/RocketPy-Team/RocketPy/actions/runs/6681843992

What is the point of not applying tests on the draft PRs?

  • Draft PRs should not stay open for so long,
  • Have we ever get close to the github monthly quota limit?

@Gui-FernandesBR
Copy link
Member Author

Example of project that also uses Dockerfile: https://github.com/pandas-dev/pandas/blob/main/Dockerfile

Thanks @phmbressan for mentioning this today.

@MateusStano MateusStano added this to the Release v1.X.0 milestone Nov 3, 2023
@ringsaturn
Copy link

FYI: you can try https://github.com/ringsaturn/tzfpy under Python 3.12 before timezonefinder is ready for it.

This is important because the github actions were not being activated after
modifying the .yaml files
@Gui-FernandesBR
Copy link
Member Author

Tests are passing on Python 3.12 (good news!!).

Several deprecation warnings are being raised tho, let's take a look at those in the next weeks.

@Gui-FernandesBR
Copy link
Member Author

FYI: you can try https://github.com/ringsaturn/tzfpy under Python 3.12 before timezonefinder is ready for it.

Thank you for your suggestion, your project is very nice. We will take a deeper look in the future.

Copy link

codecov bot commented Nov 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (master@59f091b). Click here to learn what that means.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #448   +/-   ##
=========================================
  Coverage          ?   69.54%           
=========================================
  Files             ?       56           
  Lines             ?     8868           
  Branches          ?        0           
=========================================
  Hits              ?     6167           
  Misses            ?     2701           
  Partials          ?        0           
Flag Coverage Δ
unittests 69.54% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Gui-FernandesBR Gui-FernandesBR merged commit 597a81a into master Nov 15, 2023
13 checks passed
@Gui-FernandesBR Gui-FernandesBR deleted the tst/update-workflows branch November 15, 2023 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C.I. Continuous Integration (Workflows and actions)
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

4 participants