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

rpcclient: expose endpoint (fixes #2912) #2915

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

ixje
Copy link
Contributor

@ixje ixje commented Feb 14, 2023

Problem

Endpoint data not accessible

Solution

expose with method as string as per #2912 (comment)

@codecov
Copy link

codecov bot commented Feb 14, 2023

Codecov Report

Merging #2915 (d9b4cf6) into master (1d6e48e) will increase coverage by 0.08%.
The diff coverage is 100.00%.

❗ Current head d9b4cf6 differs from pull request most recent head 3bdb3a8. Consider uploading reports for the commit 3bdb3a8 to get more accurate results

@@            Coverage Diff             @@
##           master    #2915      +/-   ##
==========================================
+ Coverage   84.98%   85.07%   +0.08%     
==========================================
  Files         329      329              
  Lines       42325    42327       +2     
==========================================
+ Hits        35970    36009      +39     
+ Misses       4890     4851      -39     
- Partials     1465     1467       +2     
Impacted Files Coverage Δ
pkg/rpcclient/client.go 86.56% <100.00%> (+0.20%) ⬆️
pkg/core/blockchain.go 78.66% <0.00%> (+0.04%) ⬆️
pkg/network/message.go 95.52% <0.00%> (+1.49%) ⬆️
pkg/services/oracle/request.go 63.18% <0.00%> (+5.00%) ⬆️
pkg/services/oracle/oracle.go 87.59% <0.00%> (+14.59%) ⬆️
pkg/network/payload/mptdata.go 100.00% <0.00%> (+18.75%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ixje
Copy link
Contributor Author

ixje commented Feb 14, 2023

I don't know what's wrong with the lint job as I don't see any issues locally with the files I modified

erik@erik-linux:~/code/neo-go$ gofmt -l .
cli/smartcontract/testdata/gas/gas.go
cli/smartcontract/testdata/nameservice/nns.go
cli/smartcontract/testdata/nex/nex.go
cli/smartcontract/testdata/nonepiter/iter.go
cli/smartcontract/testdata/verifyrpc/verify.go
erik@erik-linux:~/code/neo-go$ git log -n1
commit ed9c99bd42e4d4141d91100e67bd0adcf42a8a85 (HEAD -> feat-endpoint, ixje/feat-endpoint)
Author: Erik van den Brink <git@erikvandenbrink.com>
Date:   Tue Feb 14 11:31:53 2023 +0100

    process feedback

the same files are reported on master

erik@erik-linux:~/code/neo-go$ git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
erik@erik-linux:~/code/neo-go$ gofmt -l .
cli/smartcontract/testdata/gas/gas.go
cli/smartcontract/testdata/nameservice/nns.go
cli/smartcontract/testdata/nex/nex.go
cli/smartcontract/testdata/nonepiter/iter.go
cli/smartcontract/testdata/verifyrpc/verify.go

@ixje ixje requested review from roman-khimov and removed request for fyrchik and AnnaShaleva February 14, 2023 13:23
@roman-khimov
Copy link
Member

OK for me, can you squash these commits?

@ixje
Copy link
Contributor Author

ixje commented Feb 14, 2023

I can but is that any different from using the "squash and merge" on github? or don't you use the web for merging? Just curious :)

example
Screenshot 2023-02-14 at 16 06 34

@roman-khimov
Copy link
Member

Yes, GH squash creates nonsense commit messages most of the time. I do care a lot about commits having clear scope and descriptive messages, my expectations with respect to that are somewhat close to the way patches are handled in Linux (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/5.Posting.rst#n60). Github is not very friendly to this workflow, unfortunately (unlike Gitlab, for example, that handles rebases much better allowing to diff versions or rebased PR), but clear commit history is too important. So we create a set of proper commits (this one is very simple, so it only require one patch) in PRs and then merge them.

@ixje
Copy link
Contributor Author

ixje commented Feb 15, 2023

Yes, GH squash creates nonsense commit messages most of the time.

You can edit the commit message before merging (similar to an interactive rebase). The only difference is you need to do it (and not forget to delete random messages) instead of the PR author. Anyway, I combined the commits and will have a look at your link. Thanks!

@roman-khimov roman-khimov merged commit 9f9c6f5 into nspcc-dev:master Feb 15, 2023
@ixje ixje deleted the feat-endpoint branch February 15, 2023 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants