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: error when v -0 is present and custom -b is passed && upgrade CID to V1 when custom -b is present and -v is not specified #10143

Merged
merged 3 commits into from
Sep 27, 2023

Conversation

patrickReiis
Copy link
Contributor

This fixes #9556
I'm some sort of beginner when it comes to contributing to open source. I think the tests could be better (I just don't know how), especially the second test since I couldn't read the response body to compare with the expected CID. So in the second test case the only test is really just asserting no error is returned.

I see in the pull request a message telling me to update "docs/changelogs", since I don't know what to update on it I won't touch it.

@patrickReiis patrickReiis requested a review from a team as a code owner September 22, 2023 17:03
@lidel lidel requested a review from hacdias September 25, 2023 13:15
patrickReiis and others added 2 commits September 26, 2023 14:27
fix(commands):  return error when -v 0 is present and custom -b is passed

test(commands): return error when -v 0 is present and custom -b is passed

fix(commands):  upgrade CID to V1 when passing a custom -b and no -v is specified

test(commands): upgrade CID to V1 when passing a custom -b and no -v is specified
Copy link
Member

@hacdias hacdias left a comment

Choose a reason for hiding this comment

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

Hey @patrickReiis, thanks for jumping into this! A few notes:

  1. I pushed a small fix (added baseStr != "") since you should still be able to call -v 0 without specifying a base, in which case the default is used.
  2. Please add a changelog entry to the latest changelog file listed in here: https://github.com/ipfs/kubo/blob/master/CHANGELOG.md

Then it'll likely be ready to merge!

patrickReiis added a commit to patrickReiis/kubo that referenced this pull request Sep 26, 2023
@patrickReiis
Copy link
Contributor Author

Hi @hacdias , the current latest is changelog is 0.23 however there's no changelog list in it like changelog 0.22 has for example, so I'll be the first one to add a changelog entry. I'm just warning because I'm not so sure if it's the right thing to do.

Copy link
Member

@hacdias hacdias left a comment

Choose a reason for hiding this comment

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

After taking a better look, we will skip the changelog entry for this kind of fix, so no worries about it. Thanks for the PR!

@hacdias hacdias added the skip/changelog This change does NOT require a changelog entry label Sep 27, 2023
@hacdias hacdias merged commit de173df into ipfs:master Sep 27, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does NOT require a changelog entry
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

cid command: should error on CID v0 with multibase prefix
2 participants