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

Fix inconsistencies in tutorial 2 #478

Merged
merged 4 commits into from
May 26, 2022
Merged

Fix inconsistencies in tutorial 2 #478

merged 4 commits into from
May 26, 2022

Conversation

mroavi
Copy link
Contributor

@mroavi mroavi commented Apr 12, 2022

Venus should revolve faster than the Earth around the sun.
This PR fixes this.
The frames argument was not being passed to the Venus Object.
An inconsistency in the text is also fixed.
The gifs have been re-generated with the fixed code.

PR Checklist

  • Did I update CHANGELOG.md with whatever changes/features I added with this PR?
  • Did I make sure to only change the part of the file where I introduced a new change/feature?
  • Did I cover all corner cases to be close to 100% test coverage (if applicable)?
  • Did I properly add Javis dependencies to the Project.toml + set an upper bound of the dependency (if applicable)?
  • Did I properly add test dependencies to the test directory (if applicable)?
  • Did I check relevant tutorials that may be affected by changes in this PR?
  • Did I clearly articulate why this PR was made the way it was and how it was made?

- Venus should revolve faster than the Earth around the sun
@mroavi
Copy link
Contributor Author

mroavi commented Apr 12, 2022

I noticed that not passing the frames to Object is not an issue since it reuses the value passed to the previous Object. However, this is not explained in this tutorial but rather later in tutorial 3. Hence, I think it is better to pass it explicitly here. This means that the real issue of why the Earth was revolving faster than Venus was because of outdated gifs.

@Ved-Mahajan
Copy link
Contributor

@mroavi Hey, yeah thats correct indeed. I did not notice that while making the gif. The gif just so cool that I really did not bother to check all that!

Thanks a lot for noticing that and fixing it!!!

@Wikunia
Copy link
Member

Wikunia commented May 26, 2022

Thanks @mroavi and sorry for the late merge.

@Wikunia Wikunia merged commit d3af5a6 into JuliaAnimators:main May 26, 2022
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.

3 participants