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

peng, billy/696 monorepo spike #722

Closed
wants to merge 14 commits into from
Closed

Conversation

nylira
Copy link
Contributor

@nylira nylira commented May 21, 2018

Issue

Closes #696

  • move voyager into subfolder
  • fix CI for voyager
  • add cosmos/explorer to new subfolder
  • passing CI for cosmos explorer (yarn build for now)
  • add interchain.io
  • add cosmos.network
  • add tendermint.com
  • add ether.peg.zone
  • add ethermint.peg.zone
  • add tendermint-vue
  • add allinbits.com

@nylira nylira requested review from faboweb and NodeGuy as code owners May 21, 2018 05:42
@nylira nylira force-pushed the peng/696-monorepo-spike branch from a09fea5 to 1756d6f Compare May 21, 2018 07:19
@nylira
Copy link
Contributor Author

nylira commented May 21, 2018

I'm using the cool Workflows feature in CircleCI to run continuous integration for our multiple projects in parallel. The more $$$ we put into CircleCI, the more parallel builds.

@nylira nylira force-pushed the peng/696-monorepo-spike branch from 67c417b to ca65385 Compare May 21, 2018 07:58
@codecov
Copy link

codecov bot commented May 21, 2018

Codecov Report

Merging #722 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #722   +/-   ##
========================================
  Coverage    86.95%   86.95%           
========================================
  Files           94       94           
  Lines         1571     1571           
  Branches        71       71           
========================================
  Hits          1366     1366           
  Misses         193      193           
  Partials        12       12
Impacted Files Coverage Δ
app/src/renderer/components/common/PageProfile.vue
app/src/renderer/components/staking/PanelSort.vue
...rc/renderer/components/common/NiSessionWelcome.vue
app/src/renderer/components/common/NiFieldVote.vue
app/src/renderer/vuex/modules/blockchain.js
app/src/renderer/components/common/TextBlock.vue
app/src/renderer/connectors/lcdClient.js
...c/renderer/components/common/NiDataEmptySearch.vue
app/src/renderer/components/common/AppHeader.vue
app/src/renderer/connectors/rpcWrapperMock.js
... and 178 more

@nylira nylira changed the title WIP: peng, billy/696 monorepo spike peng, billy/696 monorepo spike May 21, 2018
@nylira
Copy link
Contributor Author

nylira commented May 21, 2018

This spike is ready for review. I did not add the @nylira/vue-* components to this PR, because I think there are enough changes happening here. If we decide to push this monorepo idea through, we can add extra components later.

There are nearly 120,000 lines of code being added in this PR. I understand this is a huge amount of maintenance and responsibility that will be added to the UI team. However, this is a good thing, because before today, I was the only one maintaining all of it. It's too much for one person. There's a lot of low-hanging fruit we can pick by merging components and updating Vue (I expect we can easily cut down the LOC by 30%).

To save you guys time during review, only two files changes have affected Voyager. ./circleci/config.yml and ./voyager/.gitignore. I've updated the CircleCI config to run multiple jobs, and I've removed app/networks from `./voyager/.gitignore to fix two otherwise failing tests.

If we want to use this, this is high priority merge. Because otherwise it'll be a huge headache keeping this PR update to date with edits to our website repos.

@nylira
Copy link
Contributor Author

nylira commented May 21, 2018

To make this PR pass CI, you'll need to edit the Branches Settings page to something like this:

screen shot 2018-05-21 at 11 21 01 pm

Because there is no longer a default CircleCI test.

@NodeGuy
Copy link
Contributor

NodeGuy commented May 21, 2018

Exciting!

@okwme
Copy link
Contributor

okwme commented May 22, 2018

also worked on the mono repo spike, but should have coordinated better w peng. might have some redundant work here.

i used a repo called tomono to combine the git history of all the repos. they're all located in a packages dir. switching branches will only show the relevant packages (and those who share a branch name like develop). the top level has a private package.json for yarn to enable workspaces. workspaces is meant to hoist shared modules to the top level in order to allow packages to share them. package specific modules are installed inside the package's node_modules dir.

anyway this is my takeaway:

mono pros

  • shared components don't fall out of sync
  • can easily include elements of explorer into voyager
  • issues will be cosmos focused instead of voyager focused (could also be con)
  • shared modules limit redundant packages on a machine (if workspaces work)

mono cons

  • any auto-deploys will have to be further configured
  • releases for voyager might not be clear compared to other releases
  • branches might get numerous/confusing
  • issues / projects might get numerous/confusing
  • couldn't get webpack to work with yarn workspaces (shared/hoisted modules)

@nylira
Copy link
Contributor Author

nylira commented May 22, 2018

Yeah we should have communicated this better. Luckily there's not much duplicated work. We both tacked different parts of the problem. I worked on resolving CircleCI tests with multiple subprojects and demonstrated that webpack can handle hoisted components (vue, js, otherwise) outside of root project directories.

@nylira
Copy link
Contributor Author

nylira commented May 22, 2018

@greg-szabo We'd like your input on this Cosmos UI monorepo proposal. Putting all the websites and apps into one repository will help us significantly with integration efforts.

Will there be any deployment issues if, e.g. the cosmos.network website is stored in a monorepo, atcosmos/cosmos-ui/cosmos.network? What if we did that for all of our websites? Would you prefer having all the websites in a subfolder (e.g. cosmos-ui/websites) for organization purposes?

Any concerns on why we shouldn't go through with this? Thanks.

@NodeGuy
Copy link
Contributor

NodeGuy commented May 22, 2018

There's a massive amount of change here which is risky. I'd like to see us pursue an incremental approach, i.e., merging only two repositories at a time. This is the approach taken by tomono:

With the original three branches under our belt, we feel confident in adding a fourth repository. This can be done after work has already been done on the monorepo; there is no need to do the complete migration at once.

Regarding tomono:

Nice find!

  • Don't Stop The World: keep working in your other repositories during the migration and pull the changes into the monorepo as you go.

We don't need this and it adds complexity. I want the monorepo migration to Stop the World and to be complete at the end.

the top level has a private package.json for yarn to enable workspaces. workspaces is meant to hoist shared modules to the top level in order to allow packages to share them. package specific modules are installed inside the package's node_modules dir.

That's an optional integration step that can be spiked later. Let's take it out of this spike.

@nylira
Copy link
Contributor Author

nylira commented May 23, 2018

I know we didn't finish discussing this in the team meeting earlier, but I think we did nearly reach consensus. I'd like to get this PR in sooner than later, because all website work is blocking on this (this PR is stopping the world). I feel like waiting an entire week to discuss it again is artificially slowing down our progress. Can we reach agreement asynchronously in this PR?

Addressing Voyager concerns: Voyager is fully working in this PR. Development, testing, building, and releasing Voyager still works. The git history for Voyager is retained. The only change we have to make is to start coding one directory deeper into the project folder. Any other PRs we merge into develop before this one is gracefully handled by git merge develop.

CircleCI is working for all of the included projects. All of the included projects can be developed on and build correctly.

None of the projects have any effect on each other, they're each siloed into their own folder. We are not sharing node modules, so there are no dependency issues to disentangle.

I made a proof of concept wherein I moved one cosmos.network component into the root ./components folder, and the website builds correctly with webpack. But we don't have to start integrating components between projects yet.

There are tons of juicy integration opportunities to be made, but none of them have been started in order to keep this spike small in scope.

@greg-szabo
Copy link

Huh, seems a huge amount of work that went into this already. I'd rather not comment on something that's so much out of my scope, I would just regurgitate industry standards and opinions.

Regarding the website deployments, this is the opposite of the Jenkins CI methodology, where 1 repo ~ 1 project (or in this case one website). So we'll have to work around some of the opinions of Jenkins to support website deployments from this monorepo.

  1. What triggers a build-and-deploy process? - Currently, if a push happens in a specific repo to the develop branch, it gets deployed to the staging website of that repo. When a push happens to master, we agreed to deploy straight to the live website. In the monorepo's case, we have to define what kind of push triggers a website deployment. For example looking through the log and checking if a specific folder (the website's) has any changes. This is not straightforward in Jenkins, I'll have to write a Python script to parse the git commits - which opens up a lot of possibilities. Or just make website deployments manual.

  2. How do you build a website? - Currently the logic to build a site is built into Jenkins (yarn calls). This is a good opportunity to move it to the repository. Or maybe with the next iteration - just to keep the change smaller.

  3. Where do we keep the site-to-foldername translations? (Example: https://cosmos.network is stored at cosmos.network folder.) Maybe this should also be stored in the repo and processed during deployments.

Permisssion / separation-of-duties-wise this might bite is in the back in the long term, but this is not my call to make. I trust you talked questions like "What if I have a contractor to update a website but I don't want to give him access to Voyager?" through.

@NodeGuy
Copy link
Contributor

NodeGuy commented May 23, 2018

I'd like to get this PR in sooner than later, because all website work is blocking on this (this PR is stopping the world).

This may be a result of confusion around the purpose of a spike. A spike is an experiment and the output is learning, i.e., "What did we learn from this spike?" It should not block development and we should start a new PR for the actual monorepo if we decide to make one after evaluating the results of this experiment.

@nylira
Copy link
Contributor Author

nylira commented May 24, 2018

@greg-szabo thanks for your detailed comments.

  1. What triggers a build-and-deploy process? A major reason for the monorepo is to allow sharing of components between websites and apps. Multiple websites made be updated by at once by a component changed in the ./monorepo/components folder. A script that watches the website project folder and the components folder for hash changes will work. There are monorepo build tools that can solve this for us too.

  2. How do you build a website? It'll be the same as before: cd ./monorepo/cosmos.network && yarn && yarn build

  3. Where do we keep the site-to-foldername translations? Do you mean the folder where all the built files, including index.html are stored? It will be stored at ./monorepo/cosmos.network/dist.

Contractors would have to go through the same issue -> pr -> review -> merge workflow that we currently do, and all of these repos are already public, so permissions won't be an issue.

@nylira
Copy link
Contributor Author

nylira commented May 24, 2018

@NodeGuy Good point, we should make a final PR after the discussion.

@greg-szabo
Copy link

Regarding the last point ("site-to-foldername translations"), it's a bit more complicated. Assume the following workflow:

  1. Git repo changes, triggers a deployment job.
  2. Deployment job executes, does git pull, gets the changelog.
  3. Script goes through the changes (here's the logic that I'm missing) and triggers website builds based on which folders changed.

The logic you're describing is that

  1. if anything in monorepo/components/ or cosmos.network/ changes, AND we're on the develop branch, trigger a deployment to CloudFront ID X.
  2. if anything in monorepo/components/ or cosmos.network/ changes, AND we're on the master branch, trigger a deployment to CloudFront ID Y.
  3. if anything in monorepo/components/ or aib.com/ changes, AND we're on the develop branch, trigger a deployment to CloudFront ID Z.
    Etc...

The above seems like a business logic that the team should be able to manage. The script doing the deployments should be able to parse the rules and create deployments based on that. This could a toml file, csv, json, whatever that describes how we do deployments and along what requirements.

This is important because if the team wants to introduce a new folder that is required for deployment, we shouldn't need to overhaul the build process because of that. A simple addition into this business logic file should be able to manage any extra requests. It also keeps dependencies clear.

@nylira
Copy link
Contributor Author

nylira commented Jun 11, 2018

Closing this discussion until after launch. Let's go with a shared component repository for now.

Close #696

@nylira nylira closed this Jun 11, 2018
@faboweb faboweb deleted the peng/696-monorepo-spike branch January 23, 2019 21:22
faboweb pushed a commit that referenced this pull request Jun 2, 2020
* adjust emoney default gas estimate

* make emoney fees safer

* changelog
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.

SPIKE: Combine cosmos/voyager, cosmos/explorer, websites, and ni-* components in monorepo
4 participants