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

refactor(rpc): change 'connected_duration' duration unit to milliseconds #3028

Merged
merged 1 commit into from
Sep 14, 2021
Merged

refactor(rpc): change 'connected_duration' duration unit to milliseconds #3028

merged 1 commit into from
Sep 14, 2021

Conversation

chanhsu001
Copy link
Contributor

@chanhsu001 chanhsu001 commented Sep 12, 2021

What problem does this PR solve?

make duration unit consistent in get_peers RPC

Problem Summary:

What is changed and how it works?

Proposal: in 'get_peer' RPC, connected_duration uses secs but last_ping_duration uses millis, we need to change connected_duration uses millis.

What's Changed:

Related changes

  • PR to update owner/repo:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

  • rpc client should change 'connected_duration' duration unit from 'second' to 'millisecond' when get_peers RPC is used.

Release note

**BREAKING RPC**. Change field `connected_duration` time unit from seconds to **milliseconds** in `get_peers` RPC to make time unit  used  consistently in `get_peers` , RPC clients are suggested to modify time quantity correspond with this change.

@chanhsu001 chanhsu001 requested a review from a team as a code owner September 12, 2021 12:06
…n 'get_peers' RPC

BREAKING CHANGE: rpc client should change 'connected_duration' duration unit from 'second' to
'millisecond' when this PR is merged.
Copy link
Member

@doitian doitian left a comment

Choose a reason for hiding this comment

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

This is a breaking change, please add the release note.

@chanhsu001
Copy link
Contributor Author

chanhsu001 commented Sep 14, 2021

This is a breaking change, please add the release note.

i already add a note at end of PR content, is it ok?

@doitian
Copy link
Member

doitian commented Sep 14, 2021

This is a breaking change, please add the release note.

i already add a note at end of PR content, is it ok?

We'll use the content inside the release note to generate the CHANGELOG.

fix(rpc): foo foo #3028

```release-note
bar bar
```

Will generate an entry like this:

- #3028: fix(rpc): foo foo

    bar bar

@doitian doitian changed the title fix(rpc): change 'connected_duration' duration unit to milliseconds refactor(rpc): change 'connected_duration' duration unit to milliseconds Sep 14, 2021
bors bot added a commit that referenced this pull request Sep 14, 2021
3028: refactor(rpc): change 'connected_duration' duration unit to milliseconds r=chanhsu001 a=chanhsu001

<!--
Thank you for contributing to nervosnetwork/ckb!

If you haven't already, please read [CONTRIBUTING](https://github.com/nervosnetwork/ckb/blob/develop/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed
-->

### What problem does this PR solve?

make duration unit consistent in `get_peers` RPC 

Problem Summary:

### What is changed and how it works?

Proposal: in 'get_peer' RPC, connected_duration uses secs but last_ping_duration uses millis,  we need to change connected_duration uses millis.

What's Changed: 

### Related changes

- PR to update `owner/repo`:
- Need to cherry-pick to the release branch

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test

Side effects

- rpc client should change 'connected_duration' duration unit from 'second' to 'millisecond' when `get_peers` RPC is used.

### Release note <!-- Choose from None, Title Only and Note. Bugfixes or new features need a release note. -->

```release-note
**BREAKING RPC**. Change field `connected_duration` time unit from seconds to **milliseconds** in `get_peers` RPC to make time unit  used  consistently in `get_peers` , RPC clients are suggested to modify time quantity correspond with this change.
```


Co-authored-by: chanhsu001 <[email protected]>
@bors
Copy link
Contributor

bors bot commented Sep 14, 2021

This PR was included in a batch that successfully built, but then failed to merge into develop. It will not be retried.

Additional information:

{"message":"At least 2 approving reviews are required by reviewers with write access.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@quake
Copy link
Member

quake commented Sep 14, 2021

bors r=quake,doitian

@bors
Copy link
Contributor

bors bot commented Sep 14, 2021

Build succeeded:

@bors bors bot merged commit f947ae2 into nervosnetwork:develop Sep 14, 2021
@chanhsu001 chanhsu001 deleted the duration_units branch September 15, 2021 02:27
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