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(share): left pad nid in NewNamespaceV0 instead of right padding #2341

Merged
merged 4 commits into from
Jun 8, 2023

Conversation

distractedm1nd
Copy link
Collaborator

Closes #2320

@distractedm1nd distractedm1nd added area:shares Shares and samples kind:fix Attached to bug-fixing PRs labels Jun 7, 2023
@distractedm1nd distractedm1nd self-assigned this Jun 7, 2023
walldiss
walldiss previously approved these changes Jun 7, 2023
@Wondertan
Copy link
Member

Once we are on it and there is no rush, let's add a unit test that confirms that we correctly left pad it. The example in the issue would be the best.

@distractedm1nd
Copy link
Collaborator Author

Added unit tests and improved docs

@codecov-commenter
Copy link

Codecov Report

Merging #2341 (ea0b9f2) into main (ae8db72) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2341      +/-   ##
==========================================
+ Coverage   50.84%   50.94%   +0.09%     
==========================================
  Files         154      154              
  Lines        9743     9744       +1     
==========================================
+ Hits         4954     4964      +10     
+ Misses       4358     4347      -11     
- Partials      431      433       +2     
Impacted Files Coverage Δ
share/nid.go 76.92% <100.00%> (+76.92%) ⬆️

... and 3 files with indirect coverage changes

@distractedm1nd distractedm1nd merged commit 1cab8eb into main Jun 8, 2023
@distractedm1nd distractedm1nd deleted the leftpad-namespace branch June 8, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:shares Shares and samples kind:fix Attached to bug-fixing PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

share: NewNamespaceV0 should be left padded
4 participants