cli icon indicating copy to clipboard operation
cli copied to clipboard

Feature:(Issue 1334) Add support for uint64slices

Open dearchap opened this issue 2 years ago • 2 comments

What type of PR is this?

  • feature

What this PR does / why we need it:

Add support for uint64 slices

Which issue(s) this PR fixes:

Special notes for your reviewer:

Please focus on altsrc changes as well, especially the type switch

Testing

Ran unit tests

Release Notes

Add support for uint64slice flag 

dearchap avatar Apr 26 '22 23:04 dearchap

@dearchap Are you up for addressing the conflicts and getting the test coverage above the threshold? No worries if not. I'm happy to take this work to completion.

meatballhat avatar Jun 18 '22 14:06 meatballhat

@meatballhat Let me take a look

dearchap avatar Jul 05 '22 12:07 dearchap

@meatballhat Done. Can you take a look ?

dearchap avatar Aug 13 '22 01:08 dearchap

@meatballhat Can you take a look at why tests are failing ? I dont quite understand why.

dearchap avatar Aug 16 '22 13:08 dearchap

@dearchap looks like tests are happy now -- coverage thresholds are still grumpy, but I don't want to be draconian about those checks. Maybe add some coverage threshold wiggle room as part of this PR? e.g.:

# in .github/codecov.yml
coverage:
  threshold: 5%

meatballhat avatar Sep 05 '22 14:09 meatballhat

@meatballhat I update the coverage threshold but it still fails. Can you approve and merge ?

dearchap avatar Sep 06 '22 13:09 dearchap

@meatballhat Can you take a look at why tests are failing ? I dont quite understand why.

@dearchap codecov will use the coverage from the base commit. Checks show target coverage need 83.22. You can try this specify a target. Would 5% be too low? I feel more coverage can reduce review time.

Dokiys avatar Sep 08 '22 03:09 Dokiys

@meatballhat I've fixed the codecov issues. I'm going to merge.

dearchap avatar Sep 10 '22 13:09 dearchap