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 math.MaxUint64 instead of 0xffffffffffffffff #5294

Closed
wants to merge 2 commits into from
Closed

Use math.MaxUint64 instead of 0xffffffffffffffff #5294

wants to merge 2 commits into from

Conversation

majorteach
Copy link
Contributor

@majorteach majorteach commented Mar 23, 2024

Which problem is this PR solving?

  • Use the variables included in the go standard library to make the code clearer

Description of the changes

  • Math MaxUint64 is a constant currently defined in the go standard library and can be used directly without having to define it repeatedly

How was this change tested?

  • no need

Checklist

Signed-off-by: majorteach <csgcgl@126.com>
@majorteach majorteach requested a review from a team as a code owner March 23, 2024 09:44
@james-ryans
Copy link
Contributor

Hmm.. is there any explanation why is this PR needed? Maybe adding the "Which problem is this PR solving?" will clarify things, because I can't see what is this PR trying to solve. 🤔

@majorteach
Copy link
Contributor Author

Hmm.. is there any explanation why is this PR needed? Maybe adding the "Which problem is this PR solving?" will clarify things, because I can't see what is this PR trying to solve. 🤔

Thanks. Modified

@yurishkuro yurishkuro added the changelog:skip Trivial change that does not require an entry in CHANGELOG label Mar 24, 2024
@yurishkuro yurishkuro changed the title using math.MaxUint64 instead of 0xffffffffffffffff Use math.MaxUint64 instead of 0xffffffffffffffff Mar 24, 2024
@yurishkuro yurishkuro enabled auto-merge (squash) March 24, 2024 02:10
Copy link

codecov bot commented Mar 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.08%. Comparing base (53592b3) to head (2415c24).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5294   +/-   ##
=======================================
  Coverage   95.08%   95.08%           
=======================================
  Files         340      340           
  Lines       16626    16626           
=======================================
  Hits        15809    15809           
- Misses        628      629    +1     
+ Partials      189      188    -1     
Flag Coverage Δ
badger 13.26% <ø> (ø)
cassandra-3.x 26.44% <ø> (ø)
cassandra-4.x 26.44% <ø> (ø)
elasticsearch-5.x 21.68% <ø> (ø)
elasticsearch-6.x 21.70% <ø> (+0.01%) ⬆️
elasticsearch-7.x 21.78% <ø> (+0.01%) ⬆️
elasticsearch-8.x 21.87% <ø> (ø)
grpc 10.96% <ø> (-0.02%) ⬇️
kafka 14.73% <ø> (ø)
opensearch-1.x 21.78% <ø> (+0.01%) ⬆️
opensearch-2.x 21.77% <ø> (-0.02%) ⬇️
unittests 92.25% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@james-ryans
Copy link
Contributor

@majorteach there is a Github CI check failing, it is because your PR is opened from the main branch of your fork. As we're not allowed to directly push commits to main branch, please create another branch and open a new PR.

@majorteach
Copy link
Contributor Author

there is a Github CI check failing, it is because your PR is opened from the main branch of your fork. As we're not allowed to directly push commits to main branch, please create another branch and open a new PR.

Thanks!

@majorteach majorteach closed this by deleting the head repository Mar 24, 2024
@majorteach
Copy link
Contributor Author

@majorteach there is a Github CI check failing, it is because your PR is opened from the main branch of your fork. As we're not allowed to directly push commits to main branch, please create another branch and open a new PR.

The new PR #5295

I checkout from main and use another branch name.

Thanks

yurishkuro pushed a commit that referenced this pull request Mar 24, 2024
<!--
!! Please DELETE this comment before posting.
We appreciate your contribution to the Jaeger project! 👋🎉
-->

Use the variables included in the go standard library to make the code
clearer

The same as #5294


## Description of the changes
- Math MaxUint64 is a constant currently defined in the go standard
library and can be used directly without having to define it repeatedly

## How was this change tested?
- no need

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: majorteach <csgcgl@126.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:skip Trivial change that does not require an entry in CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants