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

Rename variable according to #555 #558

Merged
merged 4 commits into from
Oct 1, 2019
Merged

Rename variable according to #555 #558

merged 4 commits into from
Oct 1, 2019

Conversation

pBouillon
Copy link
Contributor

@pBouillon pBouillon commented Oct 1, 2019

Rename variables in the IRandom interface according to #555

@CLAassistant
Copy link

CLAassistant commented Oct 1, 2019

CLA assistant check
All committers have signed the CLA.

@pBouillon
Copy link
Contributor Author

@dahlia I'm having troubles understanding what is expected to pass the changelog check, could you help me ?

@dahlia
Copy link
Contributor

dahlia commented Oct 1, 2019

@dahlia I'm having troubles understanding what is expected to pass the changelog check, could you help me ?

@pBouillon You need to write a log about this change into Backward-incompatible interface changes section in the CHANGES.md file. It should be like:

 -  Renamed `minValue`/`maxValue` parameters to `lowerBound`/`upperBound`
    of `IRandom.Next()` methods.  [[#555], [#558]]

Also there should be reference definitions for [#555] and [#558], but you could get how to do that if you look around at other changelogs.

@pBouillon
Copy link
Contributor Author

@dahlia I'm having troubles understanding what is expected to pass the changelog check, could you help me ?

@pBouillon You need to write a log about this change into "Backward-incompatible interface changes" section in the CHANGES.md file. It should be like:

 -  Renamed `minValue`/`maxValue` parameters to `lowerBound`/`upperBound`
    of `IRandom.Next()` methods.  [[#555], [#558]]

Also there should be reference definitions for [#555] and [#558], but you could get how to do that if you look around at other changelogs.

Thank you ! It's done with the relevant links to the issue and this PR

@pBouillon
Copy link
Contributor Author

@dahlia All tests clear ! I also rebased my fork from the remote master branch, do I need to add anything else to this PR ? 😄

@dahlia
Copy link
Contributor

dahlia commented Oct 1, 2019

@pBouillon Sorry, but could you rebase your changes on the current master and move your changelogs from 0.6.0 to 0.7.0?

@pBouillon
Copy link
Contributor Author

Of course @dahlia, rebase done !

@dahlia
Copy link
Contributor

dahlia commented Oct 1, 2019

@pBouillon We just released new version (0.6.0) few hours ago, so your change should be included in the next release (0.7.0). So could you please move changelogs you wrote from Version 0.6.0 to Version 0.7.0?

@pBouillon
Copy link
Contributor Author

All set @dahlia, let me know if you would like any other modification

CHANGES.md Outdated Show resolved Hide resolved
@dahlia
Copy link
Contributor

dahlia commented Oct 1, 2019

/rebase

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #558 into master will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #558      +/-   ##
==========================================
+ Coverage   90.62%   90.65%   +0.03%     
==========================================
  Files         201      201              
  Lines       15116    15116              
==========================================
+ Hits        13699    13704       +5     
+ Misses       1131     1126       -5     
  Partials      286      286
Impacted Files Coverage Δ
Libplanet/Net/Protocols/KademliaProtocol.cs 65.64% <0%> (+1.39%) ⬆️

@dahlia
Copy link
Contributor

dahlia commented Oct 1, 2019

@pBouillon Okay, everything seems alright! We are going to merge this patch immediately after the build passes. Thanks you for the contribution and following our picky requests. 🙏

@pBouillon
Copy link
Contributor Author

Alright ! Thank you for your reactivity about this PR ! 😃

@dahlia dahlia requested a review from a team October 1, 2019 11:17
@dahlia dahlia added this to the 0.7.0 milestone Oct 1, 2019
@dahlia dahlia merged commit 607ca06 into planetarium:master Oct 1, 2019
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.

4 participants