Skip to content

MOVE ONLY: move rpc* to rpc/#7348

Merged
laanwj merged 2 commits intobitcoin:masterfrom
dcousens:master
Jan 28, 2016
Merged

MOVE ONLY: move rpc* to rpc/#7348
laanwj merged 2 commits intobitcoin:masterfrom
dcousens:master

Conversation

@dcousens
Copy link
Contributor

@dcousens dcousens commented Jan 15, 2016

Was just playing around with the RPC, and found the constant prefixing of file names frustrating.

This also seems to be in line with the way other modules (zmq etc) have been grouped.

Should be easy to review.

Note: This won't negatively impact git blame

@maflcko
Copy link
Member

maflcko commented Jan 15, 2016

What's up with src/wallet/rpc*?

maflcko
maflcko reviewed Jan 15, 2016
View reviewed changes
src/rpc/server.h Outdated
Copy link
Member

@maflcko maflcko Jan 15, 2016

Choose a reason for hiding this comment

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

There is no rpc/wallet.cpp?!

Copy link
Contributor Author

@dcousens dcousens Jan 15, 2016

Choose a reason for hiding this comment

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

Indeed.
I wonder what the best course of action is here?
I'll probably just change that to wallet/rpcwallet.cpp

Copy link
Contributor

@jonasschnelli jonasschnelli Jan 15, 2016

Choose a reason for hiding this comment

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

#7307 would solve that.
But I agree, changing to wallet/rpcwallet.cpp is probably fine for this PR.

Copy link
Member

@laanwj laanwj Jan 16, 2016

Choose a reason for hiding this comment

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

Yes, my suggested fix too would b to just fix the comment

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Jan 15, 2016

Nice!
Concept ACK.

@laanwj
Copy link
Member

laanwj commented Jan 16, 2016

Concept ACK.

Not that you moved it, but for completeness' sake: please leave rpcwallet.cpp in src/wallet. e.g. it is the RPC interface to the wallet. This, imo, logically belongs to the wallet: if there was a different wallet it could have a different RPC interface.

@dcousens
Copy link
Contributor Author

dcousens commented Jan 17, 2016

@laanwj amended inline comment commit. Should be good to go.

This, imo, logically belongs to the wallet: if there was a different wallet it could have a different RPC interface.

Agreed.

@maflcko
Copy link
Member

maflcko commented Jan 17, 2016

@dcousens Could you make the commits atomic, so that each of them compiles?

@dcousens
Copy link
Contributor Author

dcousens commented Jan 17, 2016

@MarcoFalke rebased and fixed a bad path in /src/qt/ (I don't typically build with Qt).
Commits were already atomic.

Waiting on travis.

@maflcko
Copy link
Member

maflcko commented Jan 17, 2016

Concept ACK 611508c

@laanwj
Copy link
Member

laanwj commented Jan 20, 2016

Needs rebase after #7307

@dcousens
Copy link
Contributor Author

dcousens commented Jan 20, 2016

Rebased

@maflcko
Copy link
Member

maflcko commented Jan 20, 2016

utACK d13f65e

@paveljanik
Copy link
Contributor

paveljanik commented Jan 23, 2016

ACK https://github.com/dcousens/bitcoin/commit/d13f65ebac13ec18b7eb55176c31f1404f185c0c

Header guards do not match now (_ missing), but not that important.

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Jan 23, 2016

utACK d13f65e.
Confirm move only.

@laanwj laanwj merged commit d13f65e into bitcoin:master Jan 28, 2016
laanwj added a commit that referenced this pull request Jan 28, 2016
@laanwj
Merge #7348: MOVE ONLY: move rpc* to rpc/
62f2d76 
d13f65e rpc: update inline comments to refer to new file paths (Daniel Cousens)
a0eaff8 move rpc* to rpc/ (Daniel Cousens)
@jtimon
Copy link
Contributor

jtimon commented Jan 28, 2016

nice

@OlegGirko OlegGirko mentioned this pull request Jul 2, 2017
Merged
@dagurval dagurval mentioned this pull request Jan 29, 2018
Merged
@str4d str4d mentioned this pull request Jun 20, 2018
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Jun 20, 2018
@zkbot
Auto merge of #3350 - str4d:rpc-reorg, r=<try>
52f4abc 
RPC code refactor

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7307
- bitcoin/bitcoin#7348
- bitcoin/bitcoin#7766
zkbot added a commit to zcash/zcash that referenced this pull request Jul 18, 2018
@zkbot
Auto merge of #3350 - str4d:rpc-reorg, r=<try>
5978dfe 
RPC code refactor

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7307
- bitcoin/bitcoin#7348
- bitcoin/bitcoin#7766
zkbot added a commit to zcash/zcash that referenced this pull request Jul 18, 2018
@zkbot
Auto merge of #3350 - str4d:rpc-reorg, r=bitcartel
cd0484e 
RPC code refactor

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#7307
- bitcoin/bitcoin#7348
- bitcoin/bitcoin#7766
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Refactoring RPC/REST/ZMQ

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@dcousens @maflcko @jonasschnelli @laanwj @paveljanik @jtimon