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

Maven multi module #359 #400

Merged
merged 7 commits into from
Nov 26, 2019
Merged

Maven multi module #359 #400

merged 7 commits into from
Nov 26, 2019

Conversation

apeksharma
Copy link
Contributor

@apeksharma apeksharma commented Nov 23, 2019

Two modules: mirror-importer and mirror-rest
Updated docs
Update CircleCI
Update deploy script and service name

Tested:

  1. ./mvnw package
  2. BUCKET=hedera-mainnet-streams docker-compose updocker-compose down

Signed-off-by: Apekshit Sharma apekshit.sharma@hedera.com

Detailed description:
Circle CI run with artifacts: https://app.circleci.com/github/hashgraph/hedera-mirror-node/pipelines/34945256-21fa-4343-be9e-3d80e6700da5/workflows/194047fe-892a-4fc4-a9f9-c22a1edaa6bf

Which issue(s) this PR fixes:
Fixes #359

Special notes for your reviewer:

Checklist

  • Documentation added
  • Tests updated

Two modules: mirror-importer and mirror-api-rest
Updated docs
Update CircleCI
Update deploy script and service name

Tested:
1. ./mvnw package
2. BUCKET=hedera-mainnet-streams docker-compose updocker-compose down

Signed-off-by: Apekshit Sharma <apekshit.sharma@hedera.com>
.circleci/config.yml Outdated Show resolved Hide resolved
Signed-off-by: Apekshit Sharma <apekshit.sharma@hedera.com>
mirror-importer/pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
mirror-api-rest/pom.xml Outdated Show resolved Hide resolved
Signed-off-by: Apekshit Sharma <apekshit.sharma@hedera.com>
.circleci/config.yml Outdated Show resolved Hide resolved
mirror-importer/pom.xml Outdated Show resolved Hide resolved
mirror-importer/pom.xml Outdated Show resolved Hide resolved
mirror-importer/pom.xml Outdated Show resolved Hide resolved
mirror-api-rest/pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
mirror-importer/pom.xml Outdated Show resolved Hide resolved
mirror-api-rest/pom.xml Outdated Show resolved Hide resolved
@steven-sheehy

This comment has been minimized.

Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

I think we should move all Java source under a importer package to avoid package collisions between maven modules. com.hedera.mirror.importer

@codecov
Copy link

codecov bot commented Nov 23, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@55fafb1). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #400   +/-   ##
=========================================
  Coverage          ?   57.67%           
=========================================
  Files             ?       56           
  Lines             ?     2474           
  Branches          ?      323           
=========================================
  Hits              ?     1427           
  Misses            ?      900           
  Partials          ?      147
Impacted Files Coverage Δ
...com/hedera/mirror/parser/balance/NumberedLine.java 75% <ø> (ø)
...java/com/hedera/mirror/domain/TransactionType.java 33.33% <ø> (ø)
...ava/com/hedera/mirror/parser/ParserProperties.java 100% <ø> (ø)
...a/com/hedera/mirror/config/CacheConfiguration.java 100% <ø> (ø)
...era/mirror/parser/event/EventStreamFileParser.java 2.58% <ø> (ø)
.../java/com/hedera/mirror/domain/ContractResult.java 83.33% <ø> (ø)
...c/main/java/com/hedera/mirror/domain/FileData.java 66.66% <ø> (ø)
...c/main/java/com/hedera/mirror/domain/LiveHash.java 66.66% <ø> (ø)
...edera/mirror/parser/balance/BalanceFileParser.java 7.14% <ø> (ø)
...era/mirror/parser/event/EventParserProperties.java 85.71% <ø> (ø)
... and 46 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55fafb1...c0660c0. Read the comment docs.

@apeksharma
Copy link
Contributor Author

apeksharma commented Nov 23, 2019

I think we should move all Java source under a importer package to avoid package collisions between maven modules. com.hedera.mirror.importer

Sounds good. Will also make it easy to see spot inter module dependency from import list.
(update1) Since the current PR is much easier to review with all the java files folded, i'd prefer doing package renaming as separate PR for easier review.
(update2) Here's the PR: #401

Signed-off-by: Apekshit Sharma <apekshit.sharma@hedera.com>
Signed-off-by: Apekshit Sharma <apekshit.sharma@hedera.com>
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

Looking better. Let's update the docker-compose.yml to change the names and paths (mirror-db, mirror-rest, mirror-importer, /var/lib/mirror-importer). Also change the docker image names as well.

.circleci/config.yml Outdated Show resolved Hide resolved
jacoco-aggregator/pom.xml Outdated Show resolved Hide resolved
jacoco-aggregator/pom.xml Outdated Show resolved Hide resolved
mirror-importer/pom.xml Outdated Show resolved Hide resolved
mirror-importer/scripts/deploy-mirror-importer.sh Outdated Show resolved Hide resolved
mirror-importer/scripts/deploy-mirror-importer.sh Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.circleci/config.yml Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
Signed-off-by: Apekshit Sharma <apekshit.sharma@hedera.com>
Copy link
Contributor Author

@apeksharma apeksharma left a comment

Choose a reason for hiding this comment

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

Just couple minor comments above. Looks great otherwise.

@hashgraph hashgraph deleted a comment from apeksharma Nov 26, 2019
</dependencyManagement>
<build>
<plugins>
<plugin>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i just realized that my last change removing blank lines and this plugin didn't get checked in. Sorry about that.
Can you please remove this too. Thanks.

Signed-off-by: Steven Sheehy <steven.sheehy@hedera.com>
@apeksharma
Copy link
Contributor Author

apeksharma commented Nov 26, 2019

looks good. let's get it in.
Let's push a test tag to check the artifacts too, wdyt?
btw, you'll have to approve it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maven multi module
4 participants