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

ApiServer and A0BlockHeader fix #745

Merged
merged 17 commits into from Dec 17, 2018

Conversation

Projects
None yet
6 participants
@AionJayT
Copy link
Collaborator

commented Dec 10, 2018

Notice

It is not allowed to submit your PR to the master branch directly, please submit your PR to the master-pre-merge branch.

Description

Please include a brief summary of the change that this pull request proposes. Include any relevant motivation and context. List any dependencies required for this change.

  1. Fix load error happens in IntelliJ due to missing module dependency.
    2, Fix A0BlockHeader toJson method output inconsistency(only effect the dashboard API);

Fixes Issue # .

Type of change

Insert x into the following checkboxes to confirm (eg. [x]):

  • Bug fix.
  • New feature.
  • Enhancement.
  • Unit test.
  • Breaking change (a fix or feature that causes existing functionality to not work as expected).
  • Requires documentation update.

Testing

Please describe the tests you used to validate this pull request. Provide any relevant details for test configurations as well as any instructions to reproduce these results.

  • Updated the test case in A0BlockHeaderTest

Verification

Insert x into the following checkboxes to confirm (eg. [x]):

  • I have self-reviewed my own code and conformed to the style guidelines of this project.
  • New and existing tests pass locally with my changes.
  • I have added tests for my fix or feature.
  • I have made appropriate changes to the corresponding documentation.
  • My code generates no new warnings.
  • Any dependent changes have been made.

@AionJayT AionJayT added the bug label Dec 10, 2018

@AionJayT AionJayT added this to the 0.3.3 milestone Dec 10, 2018

@AionJayT AionJayT self-assigned this Dec 10, 2018

@aion-kelvin
Copy link
Collaborator

left a comment

Looks good! Some small suggestions for code style in the tests, but not a blocker

@@ -129,6 +132,7 @@ public void testBlockHeaderFromUnsafeSource() throws Exception {
assertThat(header.getSolution()).isEqualTo(new byte[1408]);
assertThat(header.getNonce()).isEqualTo(ByteUtil.EMPTY_WORD);
assertThat(header.getDifficulty()).isEqualTo(ByteUtil.EMPTY_HALFWORD);
assertThat(header.getVersion() == (byte) 0x01);

This comment has been minimized.

Copy link
@aion-kelvin

aion-kelvin Dec 11, 2018

Collaborator

style: assertThat(header.getVersion()).isEqualTo((byte) 0x01);

This comment has been minimized.

Copy link
@AionJayT

AionJayT Dec 17, 2018

Author Collaborator

the getVersion is returning byte, doesn't have the isEqualTo method.

@@ -165,6 +170,7 @@ public void testBlockHeaderFromRLP() throws Exception {
assertThat(reconstructed.getParentHash()).isEqualTo(header.getParentHash());
assertThat(reconstructed.getNonce()).isEqualTo(header.getNonce());
assertThat(reconstructed.getDifficulty()).isEqualTo(header.getDifficulty());
assertThat(reconstructed.getVersion() == header.getVersion());

This comment has been minimized.

Copy link
@aion-kelvin

aion-kelvin Dec 11, 2018

Collaborator

style: assertThat( ... ).isEqualTo( ... );

This comment has been minimized.

Copy link
@AionJayT

AionJayT Dec 17, 2018

Author Collaborator

some here.

beidouz and others added some commits Nov 29, 2018

@AionJayT AionJayT merged commit 3b2e096 into master-pre-merge Dec 17, 2018

1 check failed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details

@AionJayT AionJayT deleted the api-fix branch Jan 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.