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

Use LLONG_MAX instead of LONG_LONG_MAX #1062

Merged
merged 2 commits into from
Mar 28, 2019

Conversation

sushichop
Copy link
Member

New Pull Request Checklist

  • I have read and understood the CONTRIBUTING guide

  • I have read the Documentation

  • I have searched for a similar pull request in the project and found none

  • I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)

  • I have added the required tests to prove the fix/feature I am adding

  • I have updated the documentation (if necessary)

  • I have run the tests and they pass

  • I have run the lint and it passes (pod lib lint)

This merge request fixes / refers to the following issues: #1061

Pull Request Description

Use LLONG_MAX instead of LONG_LONG_MAX.

It says in limit.h that you should use LLONG_MAX instead of LONG_LONG_MAX.
/* LONG_LONG_MIN/LONG_LONG_MAX/ULONG_LONG_MAX are a GNU extension. It's too bad
that we don't have something like #pragma poison that could be used to
deprecate a macro - the code should just use LLONG_MAX and friends.
*/

It says in `limit.h` that you should use `LLONG_MAX` instead of `LONG_LONG_MAX`.

/* LONG_LONG_MIN/LONG_LONG_MAX/ULONG_LONG_MAX are a GNU extension.  It's too bad
   that we don't have something like #pragma poison that could be used to
   deprecate a macro - the code should just use LLONG_MAX and friends.
 */
Copy link
Contributor

@lolgear lolgear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix it!

@OSSPolice
Copy link

1 Warning
⚠️ The library files were changed, but the tests remained unmodified. Consider updating or adding to the tests to match the library changes.

Generated by 🚫 Danger

@codecov-io
Copy link

Codecov Report

Merging #1062 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1062   +/-   ##
=======================================
  Coverage   38.94%   38.94%           
=======================================
  Files          27       27           
  Lines        4018     4018           
=======================================
  Hits         1565     1565           
  Misses       2453     2453
Impacted Files Coverage Δ
Classes/DDFileLogger.m 58.6% <100%> (+0.12%) ⬆️
Classes/DDOSLogger.m 19.23% <0%> (-46.16%) ⬇️
Tests/Tests/DDAtomicCounterTests.m 100% <0%> (+1.85%) ⬆️
Classes/DDASLLogger.m 55.76% <0%> (+42.3%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a703620...8222ddd. Read the comment docs.

@sushichop sushichop merged commit 8c3cb19 into CocoaLumberjack:master Mar 28, 2019
@sushichop sushichop deleted the PR/improve-macro branch March 28, 2019 14:27
@bpoplauschi bpoplauschi added this to the 3.5.3 milestone Apr 2, 2019
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

5 participants