pgf icon indicating copy to clipboard operation
pgf copied to clipboard

Add warning about floating-point arithmetic to `foreach` introduction

Open 3geek14 opened this issue 3 years ago • 5 comments

Signed-off-by: 3geek14 [email protected]

Motivation for this change

On #1050, @muzimuzhi suggested it might be worth adding a warning about floating-point errors in foreach loops to the tutorial where they are first introduced. I think my suggested change might be a bit cryptic about what sorts of rounding errors show up, but I don't know how explicit it's worth being here, when it's spelled out in far more detail in section 88, Repeating Things: The Foreach Statement. I also added to the example code a second loop that does have a rounding error preventing the final tick from being drawn.

Checklist

Please check the boxes below and signoff your commits to explicitly state your agreement to the Developer Certificate of Origin:

  • [ ✓ ] Code changes are licensed under GPLv2 + LPPLv1.3c
  • [ ✓ ] Documentation changes are licensed under FDLv1.2

3geek14 avatar Sep 20 '21 16:09 3geek14

Could you please write a little more succinct commit messages? Commit messages can have more than one line, so something like this would be more appropriate:

manual: add warning about floating-point arithmetic #1050

Per a suggestion from @muzimuzhi, add a warning about floating-point arithmetic
to the first introduction of `foreach` in Karl's tutorial.

See also https://www.conventionalcommits.org/, although we do not entirely practice this here (yet).

hmenke avatar Sep 20 '21 17:09 hmenke

Thanks for the link, I'll try to write better commit messages in the future (as well as try to remember to sign my commits). Since I'm not sure how to retroactively sign commits, my plan was to close this PR and submit a new one. Should I instead fix this one somehow?

3geek14 avatar Sep 20 '21 17:09 3geek14

You can run git rebase --signoff -i origin/master and replace all pick by reword to rewrite the commit messages and automatically signoff.

hmenke avatar Sep 20 '21 17:09 hmenke

Hmm applying suggested changes on github's webpage (by adding "Co-authored-by" trailer) will always not add the "Signed-off-by" trailer, which makes the check failed. Even worse when the two trailers are used together, whom the "I" used in Developer Certificate of Origin is referring would be not unique / unclear. Someone may interpret that only the committer is signed off, not including the one co-authored.

@hmenke What's your opinion in the long term?

muzimuzhi avatar Sep 25 '21 17:09 muzimuzhi

Signed-off-by is not really important for small fixes. It's more important for new features. I don't want to get trolled again by someone who doesn't understand their own licensing terms.

hmenke avatar Sep 25 '21 20:09 hmenke