Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Clarify description of blockindex #7541

Merged
merged 1 commit into from Feb 19, 2016

Conversation

Projects
None yet
8 participants
Owner

sipa commented Feb 16, 2016

utACK

Contributor

paveljanik commented Feb 16, 2016

ACK pinheadmz/bitcoin@7f01e4e
Right, using "index" instead of "position" is better here.

@dcousens dcousens and 3 others commented on an outdated diff Feb 16, 2016

src/wallet/rpcwallet.cpp
@@ -1444,7 +1444,7 @@ UniValue listtransactions(const UniValue& params, bool fHelp)
" \"trusted\": xxx (bool) Whether we consider the outputs of this unconfirmed transaction safe to spend.\n"
" \"blockhash\": \"hashvalue\", (string) The block hash containing the transaction. Available for 'send' and 'receive'\n"
" category of transactions.\n"
- " \"blockindex\": n, (numeric) The block index containing the transaction. Available for 'send' and 'receive'\n"
+ " \"blockindex\": n, (numeric) The position of the transaction in the block that includes it. Available for 'send' and 'receive'\n"
@dcousens

dcousens Feb 16, 2016

Contributor

tiny nit: position seems slightly non-conventional?

The index of the transaction in the block that includes it.

@instagibbs

instagibbs Feb 16, 2016

Member

Agreed with this slight re-phrasing

@laanwj

laanwj Feb 17, 2016

Owner

I'd prefer to stick with index too. It has a well-defined meaning, while position is too general (it could be a byte offset, or a set of 3D coordinates within the unit block...).

@pinheadmz

pinheadmz Feb 17, 2016

Contributor

cool, updated

Contributor

dcousens commented Feb 16, 2016

utACK 7f01e4e

Member

instagibbs commented Feb 17, 2016

utACK

Contributor

paveljanik commented Feb 18, 2016

ACK pinheadmz/bitcoin@ef40f6b after squash

Member

jonasschnelli commented Feb 18, 2016

ACK ef40f6b0703fb2004183b85bdd2fb9e88b58b349.

Owner

laanwj commented Feb 18, 2016

Before merging, can you please squash these changes into one commit? e.g.

$ git rebase -i 8b70a64

In the editor, replace the second 'pick' with 'f', then save and exit.
Then, force-push the result to this branch with

git push -f origin master
Contributor

pinheadmz commented Feb 18, 2016

@laanwj thanks for your help

@laanwj laanwj merged commit 7eef1d0 into bitcoin:master Feb 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Feb 19, 2016

Merge #7541: Clarify description of blockindex
7eef1d0 Clarify description of blockindex (Matthew Zipkin)
Member

MarcoFalke commented Feb 26, 2016

@laanwj I guess this could be backported to 0.12.1

Also, Post-merge ACK.

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Apr 27, 2016

Member

MarcoFalke commented Jun 9, 2016

Backported as part of #7938. Removing label 'Needs backport'.

zander added a commit to zander/bitcoinclassic that referenced this pull request Jun 16, 2016

thokon00 added a commit to faircoin/faircoin that referenced this pull request Jun 28, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment