search

buildkite / frontend

🌏 The front-end application code for https://buildkite.com
MIT License
137 stars 27 forks source link

Relay Modern Pipeline Index #711

Closed plasticine closed 5 years ago

plasticine commented 5 years ago

This patch converts the pipeline index components to Relay Modern — the first step towards our plan of shipping the pipeline index page for public builds. Functionally from a customer perspective should not be any different from the implementation it replaces.

This PR really needs some solid behavioural testing to make sure I’ve not broken any edge cases that I’m not aware of, so I’d love a few investigative reviews in addition to just eyeballing the code if possible. I’ve tried to be really careful with swapping things over when refactoring, and I’ve done a review myself, but would love fresh eyes! 🧐😍

I started poking around with adding some unitey style tests around some of these screen components but relay classic does not make that super easy to do, so these tests might have to wait until the next step of this feature, which is actually switching out the Relay runtime from Classic → Modern.

See https://github.com/buildkite/buildkite/pull/3267 for the Buildkite side of this change (though there is no actual diff there other than the frontend ref changing).

plasticine commented 5 years ago

@keithpitt did you get a chance to have a look at this sucka?

plasticine commented 5 years ago

Pivoting this in a bit of a different direction, and dark-shipping this new implementation under a hidden route so that it’s less all-in. Unfortunately there are inescapable UX issues with this change that stem from relay classic/modern stuff. We’ll still use all this work, but in the interests of keeping PRs actionable and focused I’m going to close this out and put up a new PR with that altered direction.