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

Mark Address and HashDigest as readonly #605 #610

Merged
merged 5 commits into from
Oct 24, 2019
Merged

Mark Address and HashDigest as readonly #605 #610

merged 5 commits into from
Oct 24, 2019

Conversation

FrancescoBonizzi
Copy link
Contributor

I made Address and HashDigest readonly.

@codecov
Copy link

codecov bot commented Oct 24, 2019

Codecov Report

Merging #610 into master will decrease coverage by 0.03%.
The diff coverage is 40%.

@@            Coverage Diff             @@
##           master     #610      +/-   ##
==========================================
- Coverage   90.92%   90.88%   -0.04%     
==========================================
  Files         202      202              
  Lines       15757    15761       +4     
==========================================
- Hits        14327    14325       -2     
  Misses       1131     1131              
- Partials      299      305       +6
Impacted Files Coverage Δ
Libplanet/Address.cs 90.65% <40%> (-2.68%) ⬇️
Libplanet/HashDigest.cs 96.42% <40%> (-3.58%) ⬇️

Copy link
Member

@longfin longfin left a comment

Choose a reason for hiding this comment

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

@FrancescoBonizzi Thanks for your work! I left some comments about changes.

CHANGES.md Outdated Show resolved Hide resolved
Libplanet/Address.cs Outdated Show resolved Hide resolved
Libplanet/Address.cs Outdated Show resolved Hide resolved
Libplanet/Address.cs Outdated Show resolved Hide resolved
Libplanet/HashDigest.cs Outdated Show resolved Hide resolved
Libplanet/HashDigest.cs Outdated Show resolved Hide resolved
Libplanet/HashDigest.cs Outdated Show resolved Hide resolved
@FrancescoBonizzi
Copy link
Contributor Author

@longfin I made the changes you suggested!

limebell
limebell previously approved these changes Oct 24, 2019
longfin
longfin previously approved these changes Oct 24, 2019
Copy link
Member

@longfin longfin left a comment

Choose a reason for hiding this comment

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

Great work! 💯

earlbread
earlbread previously approved these changes Oct 24, 2019
Copy link
Contributor

@earlbread earlbread left a comment

Choose a reason for hiding this comment

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

👍

@longfin
Copy link
Member

longfin commented Oct 24, 2019

@FrancescoBonizzi This PR seems to have conflicts due to merging another PR(#609). could you rebase this branch(FrancescoBonizzi:ForIssue605) onto master?

@FrancescoBonizzi
Copy link
Contributor Author

FrancescoBonizzi commented Oct 24, 2019

@longfin I rebased but It seems nothing changed.

Could you please help me? :-)

image

@longfin
Copy link
Member

longfin commented Oct 24, 2019

@longfin I rebased but It seems nothing changed.

Could you please help me? :-)

image

I guess it needs to pulling master from upstream(git@github.com:planetarium/libplanet.net.git) before rebasing.

@FrancescoBonizzi
Copy link
Contributor Author

FrancescoBonizzi commented Oct 24, 2019

@longfin I rebased but It seems nothing changed.
Could you please help me? :-)
image

I guess it needs to pulling master from upstream(git@github.com:planetarium/libplanet.net.git) before rebasing.

I did it, it doesn't work either. Maybe can I try to resolve conflicts? It seems to be just on the CHANGES row

@longfin longfin dismissed stale reviews from earlbread, limebell, and themself via 9985d4b October 24, 2019 13:08
@longfin
Copy link
Member

longfin commented Oct 24, 2019

@FrancescoBonizzi I've rebased and resolved conflict on CHNAGES.md.

If you want to make other changes on FrancescoBonizzi:ForIssue605, please use git pull --rebase to rebase your local tree too.

@FrancescoBonizzi
Copy link
Contributor Author

@FrancescoBonizzi I've rebased and resolved conflict on CHNAGES.md.

If you want to make other changes on FrancescoBonizzi:ForIssue605, please use git pull --rebase to rebase your local tree too.

Thanks a lot!

@longfin longfin merged commit 16a7e1f into planetarium:master Oct 24, 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.

5 participants