You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datasketches.apache.org by "jbapple (via GitHub)" <gi...@apache.org> on 2023/01/27 23:42:39 UTC
[GitHub] [datasketches-cpp] jbapple opened a new pull request, #338: Reduce likelihood of macro collision
jbapple opened a new pull request, #338:
URL: https://github.com/apache/datasketches-cpp/pull/338
. . . by removing one macro and prefacing the other with the logical module it's associated with.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-cpp] coveralls commented on pull request #338: Reduce likelihood of macro collision
Posted by "coveralls (via GitHub)" <gi...@apache.org>.
coveralls commented on PR #338:
URL: https://github.com/apache/datasketches-cpp/pull/338#issuecomment-1407197008
## Pull Request Test Coverage Report for [Build 4029048306](https://coveralls.io/builds/56372041)
* **0** of **0** changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **93.871%**
---
| Totals | [](https://coveralls.io/builds/56372041) |
| :-- | --: |
| Change from base [Build 4027761243](https://coveralls.io/builds/56366925): | 0.0% |
| Covered Lines: | 2328 |
| Relevant Lines: | 2480 |
---
##### 💛 - [Coveralls](https://coveralls.io)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-cpp] jmalkin commented on pull request #338: Reduce likelihood of macro collision
Posted by "jmalkin (via GitHub)" <gi...@apache.org>.
jmalkin commented on PR #338:
URL: https://github.com/apache/datasketches-cpp/pull/338#issuecomment-1407251119
I was thinking that we could alternatively restructure as a proper C++ class, but this certainly works for now and does improve naming safety. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-cpp] jbapple commented on pull request #338: Reduce likelihood of macro collision
Posted by "jbapple (via GitHub)" <gi...@apache.org>.
jbapple commented on PR #338:
URL: https://github.com/apache/datasketches-cpp/pull/338#issuecomment-1407240405
> I feel like it might be worth doing this for the other macros, too?
Done.
> Alternatively, would it make more sense to make this a namedspaced c++ thing?
`ROTL64` and `CSA` (new in this commit) could be replaced by inline functions, which could be namespaced. The others cannot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-cpp] jmalkin commented on pull request #338: Reduce likelihood of macro collision
Posted by "jmalkin (via GitHub)" <gi...@apache.org>.
jmalkin commented on PR #338:
URL: https://github.com/apache/datasketches-cpp/pull/338#issuecomment-1407234868
I feel like it might be worth doing this for the other constants, too?
Alternatively, would it make more sense to make this a namedspaced c++ thing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org
[GitHub] [datasketches-cpp] jmalkin merged pull request #338: Reduce likelihood of macro collision
Posted by "jmalkin (via GitHub)" <gi...@apache.org>.
jmalkin merged PR #338:
URL: https://github.com/apache/datasketches-cpp/pull/338
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datasketches.apache.org
For additional commands, e-mail: commits-help@datasketches.apache.org