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

fix some golangci-lint warnings #144

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

pranjalkole
Copy link
Contributor

  • change the tv parameter in the cborRoundTripper and jsonRoundTripper functions to a pointer in cots/eat_cwtclaims_test.go
  • remove the unnecessary slicing in comid/instance_test.go
  • change 0 == bytes.Compare(a, b) to bytes.Equal(a, b) in cots/cas_and_tas_test.go
  • return an error if mapLen > math.MaxUint32 in encoding/cbor.go

This is a pretty small PR to get familiar with the codebase. I'm also not sure if this would be a useful PR as make presubmit gives a lot of other errors. Let me know if I should fix the others as well.

@pranjalkole
Copy link
Contributor Author

Oops, forgot to add a DCO. Anyways, waiting for a review after which I'll open a new PR with a DCO

Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

hi @pranjalkole, thanks for the contribution!

While you are right about this PR not fixing all the lint errors/warnings (see https://github.com/veraison/corim/actions/runs/12573760578/job/35188941045?pr=144#step:7:1), it's still a net improvement that is worth merging.

@thomas-fossati
Copy link
Contributor

Oops, forgot to add a DCO. Anyways, waiting for a review after which I'll open a new PR with a DCO

You don't need to open an new PR, just follow the instructions at https://github.com/veraison/corim/pull/144/checks?check_run_id=35047090555

Copy link
Contributor

@yogeshbdeshpande yogeshbdeshpande left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for your contributions

@thomas-fossati
Copy link
Contributor

thomas-fossati commented Jan 14, 2025

hi @pranjalkole, could you please fix the DCO check as per #144 (comment)

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.

4 participants