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 for bug number 4936 #37

Closed
wants to merge 1 commit into from

Conversation

nguyenla
Copy link

@nguyenla nguyenla commented Dec 9, 2016

Fixed bug number 4936 by fixing the method format_decimal() in mathlib.py.

@quozl
Copy link

quozl commented Dec 9, 2016

Thanks for your patch. I've applied your patch and calculated sin(179) and sin(181) and the problem mentioned by the bug is fixed.

Some other comments on your commit;

Commit description

  • add the problem; at the moment one has to go and read the bug report to find the problem,
  • keep first line shorter than 50 characters, as a summary, put details after two line breaks,
  • end with a link to https://bugs.sugarlabs.org/ticket/4936

Update bugs.sugarlabs.org

  • add reference to your pull request, or if you cannot then say so and we will,
  • resolve any doubt as to whether the other two pull requests mentioned were merged,

Hope that helps!

To change the commit message, you might git commit --amend then git push --force to update the pull request.

@quozl
Copy link

quozl commented Feb 15, 2017

Thanks, I've packaged a61b8de for use by OLPC.

@quozl
Copy link

quozl commented Feb 20, 2017

Should @nguyenla or shiwakoa not resubmit this pull request, then I'll presume they have abandoned work and I'll resubmit it myself.

@nguyenla
Copy link
Author

Should I submit a new and separate pull request with the same changes to the code, but with a different commit description then? Please advise me.

@quozl
Copy link

quozl commented Feb 20, 2017

Up to you really. Whatever works best for you.

You may either

  • git commit --amend to improve the commit message, then git push --force, which will update this pull request, or
  • you could clone again, and make new branch, edit, commit, push, and make pull request.

If it were me, the first option would be easier.

Our sending a pull-request guide does not mention the git commit --amend method yet. It is difficult to cover every possible combination of git task.

@quozl
Copy link

quozl commented Feb 22, 2017

Replaced by #40, please close #37.

@quozl quozl closed this May 29, 2017
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