search

usnistgov / AGA8

Files associated with the AGA8 standard
Other
36 stars 21 forks source link

Few math function tweaks and Wsign-conversion warning fix #3

Closed ornithophile closed 5 years ago

ornithophile commented 5 years ago

Hi, saw that you took a PR, have a small one of my own to submit. Nothing major, replaced ints with size_t for vector indices (Clang was giving a warning), replaced the hyperbolic functions with the standard library ones (these have been included since C90), and replaced some instances of pow(x,2) with an inline function sq.

ianhbell commented 5 years ago

Can you please confirm that this will still work in VS2008?

ianhbell commented 5 years ago

Can you also confirm that you are willing to make your contributions in the public domain?

ornithophile commented 5 years ago

Yes to public domain, and had to make a small change for VS2008 -- it works now. Forgot that cstdint is C99 and so VS2008 doesn't have it, but cstdlib has size_t.

ianhbell commented 5 years ago

I think you need to merge master into this branch because there are duplicated commits. Easiest solution might be to close this and make your changes in a new branch based on master.

ianhbell commented 5 years ago

Even better would be to make a branch in your repo for the PR. That is almost always the recommendation. It's not a great strategy to make PR based on master branch of fork (I've learned this the hard way)

ornithophile commented 5 years ago

There shouldn't be any duplicated commits, picked those out with a rebase. Just going to close this, I don't have half a day to play git gymnastics.

ianhbell commented 5 years ago

I'm sorry, I think it is my mistake. I thought that the math function things had already been merged.

ornithophile commented 5 years ago

No worries, thanks, I just totally suck at git -- usually end up spending more time on the git part than the changes in the PR. Next time I will put on a separate branch.

ianhbell commented 5 years ago

You and me both! Though I spend much of my day messing with git, I'm far from an expert.