Skip to content

Conversation

@ompro07
Copy link
Contributor

@ompro07 ompro07 commented Jun 2, 2022

The reference point for the calculation of the center of mass of the motor was altered from the geometric center of the propellant grain to the nozzle

Pull request type

Please check the type of change your PR introduces:

  • Code base additions (bugfix, features)
  • Code maintenance (refactoring, formatting, renaming, tests)
  • ReadMe, Docs and GitHub maintenance
  • Other (please describe):

Pull request checklist

Please check if your PR fulfills the following requirements, depending on the type of PR:

  • ReadMe, Docs and GitHub maintenance:

    • Spelling has been verified
    • Code docs are working correctly
  • Code base maintenance (refactoring, formatting, renaming):

    • Docs have been reviewed and added / updated if needed
    • Lint (black rocketpy) has passed locally and any fixes were made
    • All tests (pytest --runslow) have passed locally
  • Code base additions (for bug fixes / features):

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

What is the current behavior?

Enter text here...

What is the new behavior?

Enter text here...

Does this introduce a breaking change?

  • Yes
  • No

Other information

Enter text here...
The reference point for the calculation of the center of mass of the motor was altered from the geometric center of the propellant grain to the nozzle, this will disable the notebooks

The reference point for the calculation of the center of mass of the motor was altered from the geometric center of the propellant grain to the nozzle
@ompro07 ompro07 requested a review from giovaniceotto as a code owner June 2, 2022 20:27
@Gui-FernandesBR Gui-FernandesBR added the Enhancement New feature or request, including adjustments in current codes label Jun 5, 2022
@Gui-FernandesBR Gui-FernandesBR self-requested a review June 5, 2022 02:41
Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

It's indeed a good addition, but as we are requesting a breaking change I believe we should work a little bit more in some files.

Besides to above commentaries I left, I believe we also should adjust the following files to ensure they work well after approving and merging this PR, the adjustments should be short and easy to be done:

  • docs\notebooks\getting_started.ipynb
  • docs\notebooks\getting_started_colab.ipynb
  • docs\notebooks\solid_motor_class_usage.ipynb
  • opt*: docs\notebooks\dispersion_analysis\dispersion_analysis.ipynb

@Gui-FernandesBR
Copy link
Member

HELP,
some of the tests are not working, however I could not find why ,
we need all checks running again before merging

@giovaniceotto giovaniceotto self-requested a review June 16, 2022 00:20
Copy link
Member

@Gui-FernandesBR Gui-FernandesBR left a comment

Choose a reason for hiding this comment

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

Everything is fine for me!
Thank you for taking care of every detail on the changed files so we guarantee the code additions are doing great.

And btw congrats for the smart and significant development, looking forward to seeing next PR from you @ompro07 !

@Gui-FernandesBR Gui-FernandesBR merged commit c9bfed2 into hybrid-integration-v2 Jun 16, 2022
@Gui-FernandesBR Gui-FernandesBR deleted the PR-2-CM-Reference branch September 10, 2022 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request, including adjustments in current codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants