It's hard to understand the code and contribute

[AlexxNica]

Guys, I’ve been trying to reason about the code for a while but there’s so much that has changed since our launch that I think we need either a good refactoring phase where we cut some of our tech debt or build a good readme/contributing docs with the steps required to get the project up and running (with our linting configs and whatnot already configured). What do you think?

Also @SgtPooki: should we wait for the d3 upgrade PR to land before doing something like this?

Any thoughts are appreciated. 🙏

[SgtPooki]

The d3 migration will result in the removal of a lot of files. Since things will still exist on legacy.starmap.site for functionality where the d3 might introduce UX regressions, we don’t need to keep them in main. We will focus bug fixes and features on the main branch and anything issues on legacy.starmap.site will have to remain, until starmap.site main supersedes it (which hopefully will be mostly done.. legacy is just a catch-all fallback)

I would recommend waiting until after the d3 PR is landed to focus on any refactoring or cleanup, but as soon as I get all the e2e and unit tests fixed, i'm going to remove any unused code + files, and then we can start narrowing in on functionality.

One thing I want to do is completely remove backend calls for /roadmap & /pendingChild and instead have backend calls only be for getting the issue data from github. Then we can do more with client-side only calls, directly to github via unauthenticated client side calls, and lessen the chance for backend tokens being exhausted. It would also reduce the runtime of Server-Side-Functions on vercel, and limit the complexity of the backend, allowing us to be less tightly coupled to vercel which would make it easier to pivot to a different backend (or hopefully a static-only site) somehow.

---

One caveat - It should be very easy to get starmap running locally, and if you're having trouble getting it running locally, we could start a DEVELOPER-NOTES.md or similar today with instructions if there's something blocking you, but I presume you're talking more about where the logic lies, and what the intentions of certain files, or what the folder structure intentions, are.

More than anything, though, we need more unit test and e2e test coverage. I would honestly rather focus on unit+e2e coverage before any cleanup, because then we can reduce the risk of any drastic changes.

[AlexxNica]

Thanks for the context, @SgtPooki!

I would recommend waiting until after the d3 PR is landed to focus on any refactoring or cleanup, but as soon as I get all the e2e and unit tests fixed, i'm going to remove any unused code + files, and then we can start narrowing in on functionality.

Got it. How long do you think the d3 PR will take to land?

For future efforts, could we have those big changes split into smaller PRs so that we can better track them and the overall goal progress? (we could even use Starmap itself to track that, or GitHub milestones)

One thing I want to do is completely remove backend calls for /roadmap & /pendingChild and instead have backend calls only be for getting the issue data from github. Then we can do more with client-side only calls, directly to github via unauthenticated client side calls, and lessen the chance for backend tokens being exhausted. It would also reduce the runtime of Server-Side-Functions on vercel, and limit the complexity of the backend, allowing us to be less tightly coupled to vercel which would make it easier to pivot to a different backend (or hopefully a static-only site) somehow.

Awesome. I agree this is needed. I actually began a PR (https://github.com/pln-planning-tools/Starmap/pull/325) a while back to experiment with moving things to the client side. There are some notes on the PR and some insightful points from @whizzzkid. We should probably revisit that once the d3 upgrade goes through.

but I presume you're talking more about where the logic lies, and what the intentions of certain files, or what the folder structure intentions, are.

Right on point! That's where I'm having a hard time getting my head around - mainly the file structure and coding styles.

More than anything, though, we need more unit test and e2e test coverage. I would honestly rather focus on unit+e2e coverage before any cleanup, because then we can reduce the risk of any drastic changes.

Agree. Should I wait for the d3 PR merge before touching tests?

[SgtPooki]

@AlexxNica code style is now enforced and things should be much cleaner and easier to maintain. let me know if you have any other specific concerns.