Skip to content

A working subset of #1788#1875

Merged
g1itch merged 5 commits into
Bitmessage:v0.6from
g1itch:compat-short
Nov 11, 2021
Merged

A working subset of #1788#1875
g1itch merged 5 commits into
Bitmessage:v0.6from
g1itch:compat-short

Conversation

@g1itch
Copy link
Copy Markdown
Collaborator

@g1itch g1itch commented Nov 2, 2021

Hello!

This is a subset of #1788, the tests that are ready (+4 tests, including those for base58 and varint) and corresponding changes in the protocol module.

@g1itch g1itch added the test label Nov 2, 2021
@g1itch
Copy link
Copy Markdown
Collaborator Author

g1itch commented Nov 2, 2021

There is also a test for base58 in #1796: 7f213f6

Comment thread src/tests/test_addresses.py
Comment thread src/tests/test_packets.py Outdated
Copy link
Copy Markdown
Member

@PeterSurda PeterSurda left a comment

Choose a reason for hiding this comment

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

Mostly ok, just minor cleanup pls.

Comment thread src/tests/test_addresses.py Outdated
Comment thread src/tests/test_addresses.py
Copy link
Copy Markdown
Member

@PeterSurda PeterSurda left a comment

Choose a reason for hiding this comment

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

Still encodeBase58(0) is missing. Fixing negative can be fixed either in this PR or a separate one.

Comment thread src/tests/test_addresses.py Outdated
@g1itch
Copy link
Copy Markdown
Collaborator Author

g1itch commented Nov 4, 2021

Fixing negative can be fixed either in this PR or a separate one.

It's a simple mechanical fix. I prefer to merge complete tests, not stubs.

@g1itch g1itch force-pushed the compat-short branch 2 times, most recently from 391228b to 2d14867 Compare November 10, 2021 18:51
@g1itch g1itch requested a review from PeterSurda November 10, 2021 18:58
@g1itch g1itch merged commit b3577a0 into Bitmessage:v0.6 Nov 11, 2021
@g1itch g1itch deleted the compat-short branch November 20, 2021 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants