Skip to content

Implement RedisValue.Length for all underlying storage kinds #2370

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Feb 9, 2023
Merged

Implement RedisValue.Length for all underlying storage kinds #2370

merged 7 commits into from
Feb 9, 2023

Conversation

mgravell
Copy link
Collaborator

@mgravell mgravell commented Feb 9, 2023

fix #2368

  • implement Length() for other encodings (using format layout)
  • unify format code
  • switch to C# 11 for u8 strings (needed a few "scoped" modifiers adding)
  • tests for format and Length

@mgravell
fix #2368
567aaa6 
- implement Length() for other encodings (using format layout)
- unify format code
- switch to C# 11 for u8 strings (needed a few "scoped" modifiers adding)
- tests for format and Length
@mgravell mgravell requested a review from NickCraver February 9, 2023 14:11
NickCraver
NickCraver reviewed Feb 9, 2023
View reviewed changes
@NickCraver
Copy link
Collaborator

If we want to go .NET 7, will need to bump Ubuntu too it looks like, not sure what's on their images (have to run in a few but can investigate this afternoon and tidy to get unblocked if you're on other stuff)

@NickCraver
Copy link
Collaborator

NickCraver commented Feb 9, 2023 •
edited
Loading

Hmm, image manifest says 7.0.102 is on there: https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md, but GitHub/Actions are loading hella slow this morning, makes it hard to dig much more atm.

@mgravell you want 7.0.102 on Windows (AppVeyond is Windows only here) - this is the bits in https://github.com/StackExchange/StackExchange.Redis/blob/main/.github/workflows/CI.yml I think we're explicitly installing 6 and it's taking over (separate path), so need to bump to 7 in that delicious YAML

NickCraver
NickCraver approved these changes Feb 9, 2023
View reviewed changes
Copy link
Collaborator

@NickCraver NickCraver left a comment

Choose a reason for hiding this comment

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

Looking good 👍

@NickCraver NickCraver mentioned this pull request Feb 9, 2023
Closed
@mgravell mgravell merged commit 51a7d90 into main Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@NickCraver NickCraver NickCraver approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RedisValue's Length() method

2 participants

@mgravell @NickCraver