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

chore(apps/whale): refactor to jellyfish conventions #1277

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

eli-lim
Copy link
Contributor

@eli-lim eli-lim commented Mar 28, 2022

To merge after build and release workflow is ready

Then we can freeze whale and move everyone over for 100% feature parity and release stability before performing the refactor

What this PR does / why we need it:

Follow-up PR to adapt whale code / structure to jellyfish conventions:

  • Consolidate unit tests and e2e tests into __tests__ with mirrored directory structure
  • Rename most files to PascalCase
  • Shorten imports

Which issue(s) does this PR fixes?:

Additional comments?:

  • I've decided to keep the naming of _module files as it helps to distinguish the entrypoint of each module
  • Some files are prefixed with _ because I think it makes sense to distinguish their private / internal nature. An alternative is to introduce a subdirectory to separate the concerns.

Status

On hold - let's complete the migration first with 100% feature parity before we proceed with refactoring

@codeclimate
Copy link

codeclimate bot commented Mar 28, 2022

Code Climate has analyzed commit 32d5b35 and detected 170 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 59
Duplication 111

View more on Code Climate.

@netlify
Copy link

netlify bot commented Mar 28, 2022

Deploy Preview for jellyfish-defi ready!

Name Link
🔨 Latest commit 32d5b35
🔍 Latest deploy log https://app.netlify.com/sites/jellyfish-defi/deploys/6241e82eb7392800097aa01b
😎 Deploy Preview https://deploy-preview-1277--jellyfish-defi.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #1277 (32d5b35) into main (3623874) will increase coverage by 2.33%.
The diff coverage is 99.72%.

@@            Coverage Diff             @@
##             main    #1277      +/-   ##
==========================================
+ Coverage   86.50%   88.84%   +2.33%     
==========================================
  Files         331      331              
  Lines        9582     9582              
  Branches     1174     1174              
==========================================
+ Hits         8289     8513     +224     
+ Misses       1223     1018     -205     
+ Partials       70       51      -19     
Impacted Files Coverage Δ
apps/whale/src/AppConfiguration.ts 100.00% <ø> (ø)
apps/whale/src/api/FeeController.ts 100.00% <ø> (ø)
apps/whale/src/api/Subsidy.ts 91.66% <ø> (ø)
apps/whale/src/api/_core/ApiError.ts 28.57% <ø> (ø)
apps/whale/src/api/_core/ApiQuery.ts 71.42% <ø> (ø)
apps/whale/src/api/_core/ApiResponse.ts 100.00% <ø> (ø)
apps/whale/src/api/_core/ApiVersion.ts 100.00% <ø> (ø)
apps/whale/src/api/cache/GlobalCache.ts 100.00% <ø> (ø)
apps/whale/src/database/Database.ts 100.00% <ø> (ø)
apps/whale/src/database/database.spec/_fixtures.ts 100.00% <ø> (ø)
... and 165 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 3623874...32d5b35. Read the comment docs.

@eli-lim eli-lim marked this pull request as draft March 28, 2022 17:21
@github-actions
Copy link

Docker build preview for jellyfish/apps is ready!

Built with commit 60d55c9

  • ghcr.io/defich/legacy-api:pr-1277
  • ghcr.io/defich/ocean-api:pr-1277
  • ghcr.io/defich/playground-api:pr-1277

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/apps kind/chore Non feature change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants