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

Migrate to monorepo #81

Closed
patcon opened this issue Jan 13, 2020 · 21 comments
Closed

Migrate to monorepo #81

patcon opened this issue Jan 13, 2020 · 21 comments

Comments

@patcon
Copy link
Contributor

patcon commented Jan 13, 2020

Hi folks!

Has this been in the conversation before? I usually like separation, but I wonder if the fragmentation is making it harder for people who don't already understand the codebase to feel like they have a handle on what's happening.

Would it make sense to consider using a mono repo for now, to ensure all the various support scripts and environment variables and docs etc are available in one place? Could just be a quick subtree merge for now, with nothing materially changing :)

Thanks for considering! I hope you're all well!

@patcon
Copy link
Contributor Author

patcon commented Jan 13, 2020

Some points from this article

  • better testing
  • reduced complexity
  • easier reviews (for changes across repos)
  • sharing common components (scripts, docker files)
  • easy refactoring

We believe mono repos are the right choice for teams that want to ship code faster.

@crkrenn
Copy link
Contributor

crkrenn commented Feb 2, 2020

+1 for mono. I’ve been writing many scripts, or long command line commands with gnu parallel or xargs just to get the code up and running on a new machine.

I created my own “common” polis directory to synchronize .env and polis.config files. (Which, as far as I have seen never have had to diverge.

As far as tracking branches, git supports hierarchical branch names, and by convention, most pull requests can be localized in a single microservice:

Polis/server/fargate
Polis/clientParticipation/localization

Etc.

Q

@patcon patcon changed the title Discussion of mono repo Migrate to monorepo Apr 9, 2020
@patcon
Copy link
Contributor Author

patcon commented Apr 9, 2020

Migrating from https://github.com/pol-is/polis-issues/issues/128#issuecomment-611537979

@patcon +1 overall on the proposal. I still suggest/request lower case "server", "math", etc.

A benefit of keeping snyk branches is that they provide a tractable way of prioritizing work. (It is easiest to fix the oldest snyk branch first.) But, I'm happy to keep the snyk branches in my fork and would support removing them from the monorepo.

+1

A suggestion: state the importance of using "git log --follow " in the README.md because of the merge. And, do gui git clients need a similar user change?

Start pull requests after downcasing?

+1 on that being in readme

Ok, so we've now got a shared space we can "bless" as the place we're working together for the next little bit!

In pol-is-trial-balloon org, we've now:
✅ renamed prior polisCode to polisCode-crkrenn
✅ pushed the monorepo described above to a fresh fork of pol-is/polisServer, and renamed to polisCode
✅ downcased subdir names
✅ set the monorepo feature branch as default for now, though it will eventually get merged upstream as whatever we'd like (master?)
✅ deleted master for now (since it's merged into monorepo)
✅ archived merged-in forks (more as demo of how we're suggesting pol-is org should later do)
✅ pinned active repos

@crkrenn note that I've renamed your pol-is-trial-balloon/polisCode to pol-is-trial-balloon/polisCode-crkrenn, and since it's our shared space now, you'll want to ensure you don't have any old remotes configured locally, that you might push to thinking you're working on your earlier throwaway repo :)

@patcon
Copy link
Contributor Author

patcon commented Apr 9, 2020

If this looks good for initial sign off, then we can start making other changes via PRs against monorepo branch of our polisCode repo.

To be clear, my understanding is that after we agree and close this kickoff issue, we'll only be working through PRs, right? No pushing directly to monorepo?

And the goal of monorepo is to get this to point where upstream can inherit it

EDIT: E.g. rest of changes through quick PRs like this pol-is-trial-balloon#1

@crkrenn
Copy link
Contributor

crkrenn commented Apr 13, 2020

@patcon and @metasoarous, what do you see as necessary changes to pol-is-trial-balloon/monorepo before opening a pull request to pol-is/polisServer dev?

A proposal:

  1. pull request https://github.com/pol-is-trial-balloon/polisCode/tree/monorepo to https://github.com/pol-is/polisServer/tree/dev
  2. iterate with pull requests to https://github.com/pol-is/polisServer/tree/dev to meet a set of requirements
    1. run on localhost in docker containers
    2. run on localhost with compiled static content for clientParticipation and clientAdmin
    3. run on aws ec2 instance?
    4. run on heroku?
  3. tag current master branch with version 2.0.0
  4. pull request https://github.com/pol-is/polisServer/tree/dev to https://github.com/pol-is/polisServer/tree/master
  5. tag new master branch with version 3.0.0

Since neither @urakagi or I can deploy an unmodified https://github.com/pol-is/polisServer/tree/master now, I think it is really for @metasoarous to define the requirements to be met before a move from dev to master.

Thoughts?

@crkrenn
Copy link
Contributor

crkrenn commented Apr 17, 2020

I'm planning to write a readme/jupyter notebook for dev installation.

I would also request to merge polis deploy into the monorepo. Any code in deploy will need to access admin, server, etc. and, in my mind, adds unnecessary complexity.

@patcon
Copy link
Contributor Author

patcon commented Apr 17, 2020

what do you see as necessary changes to pol-is-trial-balloon/monorepo before opening a pull request to pol-is/polisServer dev? A proposal: ...

(1) def in no rush to move to working from upstream dev branch. The maintainers seem busy, and I anticipate friction in merging, which outweighs any perceived benefit of getting this upstreamed rn

(2) i. ii. 👍 iii. iv. eager for them, but happy for later merge too

(3) (5) branch-specifics aside, tagging sounds fine to me!

I think it is really for @metasoarous to define the requirements to be

Sure.

I would also request to merge polis deploy into the monorepo.

Already done :) It was just a readme that's now in docs/deployment.md

I've extracted the tasks you mentioned into separate issues, linked above.


@crkrenn oh hey, just realized: the [undeclared] success criterion of this issue in my mind was "new directory structure for monorepo exists, in a repo where we can work confidently", and i think that might be done. If we'd like more discussion to happen here (which is a-ok), might you be able to suggest a new "success criteria" for this issue, so that we know when the issue's met its purpose? Because CLOSING THINGS FEELS GREAT 🎉

@crkrenn
Copy link
Contributor

crkrenn commented Apr 18, 2020

@patcon and @metasoarous,

  • TL;DR:

  • “The maintainers seem busy”

    • I understand.
  • “I anticipate friction in merging, which outweighs any perceived benefit of getting this upstreamed”

    • I disagree. It is precisely because of anticipated friction that, I believe, it is important for this to happen sooner rather than later.
    • The merge of the monorepo to dev should be relatively low friction.
    • The more challenging questions are regarding backward compatibility, new config variable, updating node packages, and this is something that both @metasoarous and someone with node experience (@david-nadaraia, perhaps) should be in on the review process.
    • On our community call, @metasoarous said that he would be liberal in approving pull requests to the dev branch.
  • With great respect for @patcon's time, effort, and intentions, I don't really need or want to work extensively on another pol-is fork.

    • Our private fork works.
    • Our public fork is close to working.
    • Migration from our current deployment to a monorepo deployment will take some time and effort.

@patcon
Copy link
Contributor Author

patcon commented Apr 19, 2020

Ok, I suppose let's wait on a maintainer response :) I def feel all our pet concerns can be accommodated. This also seems a decent candidate for quickly hashing out on the next call

@metasoarous
Copy link
Member

Hi folks! Thanks so much for pushing this forward!

I'm all for merging into a dev branch as soon as you guys are ready 👍

My only critique so far on the repo: I'd rather the clientXxx repos be either kebab-cased client-xxx, xxx-client or just xxx. I'm not a fan of camel case for top level repo structure.

I'm also all for getting the configuration centralized in config files. Doesn't necessarily have to happen before merging to a dev branch, but feels high-priority to me.

Note that supporting heroku deploy may be a bit challenging given the monorepo approach. Also, heroku only really supports env var configuration AFAIA, so we'll either need mixed config support (with heavy recommendations for file based config where possible). I'm in support though if those issues can be addressed reasonably.

I'm not sure why we'd want a Jupyter notebook for dev installation README. Seems like just another md would be fine. Am I missing something @crkrenn?

I think it would be nice to get things running smoothly before merging dev to master. I realize this gives time for divergence, but we're not editing a lot of code at the moment, so I'm not super worried about that being a challenge.

Please let me know if I missed anything here.

Thanks again!

@patcon
Copy link
Contributor Author

patcon commented Apr 20, 2020

+1 on kebab-case. sounds good on dev branch now (let's merge first small PR there soon?). +1 on regular md for docs. +1 for config in files asap with strong recco, but also supporting envvars if possible (i'm on the envvar train myself 🤷).

ps, heroku buildpacks exist to work fine with various monorepo/subdir formats fwiw (we can kick that convo to later)

@urakagi
Copy link
Contributor

urakagi commented Apr 20, 2020

Are you only merging the main repo, or including my modifications on like https://github.com/PDIS/polisServer/tree/local-polis or https://github.com/PDIS/polisClientAdmin/tree/local-polis and so on?

@crkrenn
Copy link
Contributor

crkrenn commented Apr 20, 2020

I'm heading out the door and will respond more fully later.

@urakagi, I'd very much like to merge PDIS mods. It might be second release after a working monorepo.

@crkrenn
Copy link
Contributor

crkrenn commented Apr 20, 2020

@metasoarous, Thanks for the review!

My only critique so far on the repo: I'd rather the clientXxx repos be either kebab-cased client-xxx, xxx-client or just xxx.

I like kebab too (if needed). I would propose "polis-code", "admin", and "participation".

I'm also all for getting the configuration centralized in config files.

I'm shooting for ".env" and "polis.config.js" for the first release

Note that supporting heroku deploy may be a bit challenging given the monorepo approach.

My priorities are 1) ubuntu & 2) docker. According to @patcon, heroku is possible if there is a volunteer.

I'm not sure why we'd want a Jupyter notebook for dev installation README.

I already realized that Jupyter was overkill. Right now, I'm working on one markdown file, one config file, one install script, & one run script.

I think it would be nice to get things running smoothly before merging dev to master. I realize this gives time for divergence, but we're not editing a lot of code at the moment, so I'm not super worried about that being a challenge.

Agreed. Since there is not a lot of current activity on master, merging master changes to dev shouldn't be a problem.

@crkrenn
Copy link
Contributor

crkrenn commented Apr 20, 2020

Can this issue be migrated to polis-issues?

@patcon
Copy link
Contributor Author

patcon commented Apr 21, 2020

Can this issue be migrated to polis-issues?

Inspired me to spawn: https://github.com/pol-is/polis-issues/issues/134

@patcon
Copy link
Contributor Author

patcon commented Apr 21, 2020

would be really excited to see your work merged in near future @urakagi :)

I would propose "polis-code", "admin", and "participation".

prev client <=> server namespace feels helpfully descriptive to me

@metasoarous
Copy link
Member

Oh; One more thing. Colin and I both feel strongly on the main repo being just polis over polis-code.

Thanks everyone!

@colinmegill
Copy link
Member

+1 to moving this over sooner rather than later
+1 to integrating PDIS changes @crkrenn @urakagi and +1 to integrating PDIS changes as the first thing after establishing the monorepo

@patcon
Copy link
Contributor Author

patcon commented May 3, 2020

Ah ok, perf! I'm down for doing this first.

Issue for tracking that PDIS work is here https://github.com/pol-is/polis-issues/issues/138, if we want to do it in small units with any required review/convo (instead of giant PRs of pol-is/polisClientParticipation#44 (comment)

As for this ticket, going to review this ticket to extract any loose todos, then close it

@patcon
Copy link
Contributor Author

patcon commented May 3, 2020

ok, re-read this, and seems most everything (config, heroku, etc.) is already repped elsewhere -- spawned a new one to sorted tagging strategy convo: #66

Closing 🎉 yay

@patcon patcon closed this as completed May 3, 2020
@patcon patcon transferred this issue from pol-is/polis-deploy May 10, 2020
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

No branches or pull requests

5 participants