Confusing project layout
felixfbecker opened this issue · 7 comments
When I started working on the browser extension, I found the project layout very hard to understand. I still don't fully understand it. From the README:
app/- application code, e.g. injected onto GitHub (as a content script)
chrome/- entrypoint for Chrome extension. Includes bundled assets, background scripts, options). Where's the entry point for Firefox and Safari?
phabricator/- entrypoint for Phabricator extension. The Phabricator extension is injected by Phabricator (not Chrome) Doesn't exist?
scripts/- development scripts
test/- test code Doesn't exist?
webpack- build configs
Not mentioned:
extension- what's the difference between this andchrome?preSourcegraph.safariextension- This looks like built files, is this intended to be checked in?
One thing I would propose is putting all sources under src.
I've been trying to build time for this to iterations for months. Here's a doc that I wrote up in April to try to get this prioritized before we reached a time where more people were working on this(I knew this time would come :)
Browser Extension Restructure
Team
Problem Statement
Background/context
The browser extension is a huge driving force in getting new users onto our products. Not only that, but it is integral to getting the full Sourcegraph experience bringing our powerful features along with you wherever you go.
The extension works great, and we should not rewrite or refactor the functional parts of the code, but its time for a restructure of the code.
It is growing more difficult to add to the project and it is pretty fragile. It is too easy to make a change and accidentally break something that should be unrelated.
Goals
- Make it easier for team members(or community members once it is open source) to make changes to the code base without breaking anything
- Make it easier to test different sections of the project
- And actually add infrastructure for tests
- Will be hard to test around the browser extension api but we can still test a lot of the core logic
- Make it easier to understand
- Not only two gate keepers for the project who know how the code works and where things should go
- Make it easier to scale the project up and down as new features, requirements or restrictions come and go
- we have a lot of dead code lying around
Definition of success
- We have less bugs shipped
- Someone who wants to make changes to the browser extension code base can easily find the area of the project they are looking for
- We have tests that ensure the core logic of the project doesn’t break
Solution
Implementation
We need to restructure the code. There are an infinite number of good and bad options. This is one possible solution inspired by the lernajs mono repo architecture:
- src
- extension
- currently
browser-extension/extension
- currently
- applications
- extension
- Currently
chrome - We still do not want to separate the build processes of chrome, Firefox or safari
- Currently
- phabricator
- bitbucket
nother potential opportunities
- extension
- packages
- blob-annotators
- file-tree
- view-on-sourcegraph
- banners
- search (
src <search query>) - link
- cli (
src :<command and args>) - shared
- Anything shared between
packages
- Anything shared between
- extension
Rough roadmap, including "stretch" tasks
Link to issues
All issues will be under the browser-ext label with the May 2018 milestone.
Known concerns/risks
- Could be hard to continue feature work and make these changes in parallel
- While this isn’t a refactor, changing existing code can introduce new bugs
- Others might not agree that the new structure is not as clear as we hope
Remember that that was written in April, so things will be different but it has the overall gist of what I'm still thinking is a good structure and (I believe) addresses Felix's concerns from this issue.
The src/applications directory will likely be different based on some ideas @chrismwendt mentioned to me last week around having the concept of "code hosts" as an interface. This idea is great and will make the structure a lot cleaner and fits well with the work I'm doing with implementing support for phabricator(I'm implementing it in an abstract way and have the concept of code hosts specific to codeintellify so it'll be easy to fit in.
Haha, whoops. Sorry! I copy/pasted that comment from a markdown file used for planning which contains the team intended to work on it denoted by our slack handles. One of my coworkers, Matt King, goes by Kingy. Also, sorry to whoever @ isaac is!
No problem! We're both from New Zealand so I was confused thinking I was involved in some project I completely forgot about. Good luck :)
The release is available on:
Your semantic-release bot