-
Notifications
You must be signed in to change notification settings - Fork 3
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
add proposal for rewrite of vela ui #831
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,217 @@ | ||
# Rewrite UI in a more common framework | ||
|
||
<!-- | ||
The name of this markdown file should: | ||
|
||
1. Short and contain no more then 30 characters | ||
|
||
2. Contain the date of submission in MM-DD format | ||
|
||
3. Clearly state what the proposal is being submitted for | ||
--> | ||
|
||
| Key | Value | | ||
| :-----------: | :--------------------------------------------: | | ||
| **Author(s)** | Ryan Rampersad, Zac Skalko, James Christensen | | ||
| **Reviewers** | | | ||
| **Date** | 2023-06-30 | | ||
| **Status** | | | ||
|
||
<!-- | ||
If you're already working with someone, please add them to the proper author/reviewer category. | ||
|
||
If not, please leave the reviewer category empty and someone from the Vela team will assign it to themself. | ||
|
||
Here is a brief explanation of the different proposal statuses: | ||
|
||
1. Reviewed: The proposal is currently under review or has been reviewed. | ||
|
||
2. Accepted: The proposal has been accepted and is ready for implementation. | ||
|
||
3. In Progress: An accepted proposal is being implemented by actual work. | ||
|
||
NOTE: The design is subject to change during this phase. | ||
|
||
4. Cancelled: While or before implementation the proposal was cancelled. | ||
|
||
NOTE: This can happen for a multitude of reasons. | ||
|
||
5. Complete: This feature/change is implemented. | ||
--> | ||
|
||
## Background | ||
|
||
<!-- | ||
This section is intended to describe the new feature, redesign or refactor. | ||
--> | ||
|
||
**Please provide a summary of the new feature, redesign or refactor:** | ||
|
||
<!-- | ||
Provide your description here. | ||
--> | ||
|
||
Currently, the Vela UI is written in [Elm](https://elm-lang.org/). Elm is a domain-specific programming language for declaratively creating web browser-based graphical user interfaces. Elm is purely functional, and is developed with emphasis on usability, performance, and robustness. | ||
|
||
Elm is a fairly obscure language with a small community, anecdotally in our contributor's experience, anecdotally by way of these recent Hacker News threads about Elm [(1)](https://news.ycombinator.com/item?id=34746161) [(2)](https://news.ycombinator.com/item?id=35495910) [(3)](https://news.ycombinator.com/item?id=36272692) and statistically according to the [2022 State of JS Survey](https://2022.stateofjs.com/en-US/other-tools/#javascript_flavors) and [2023 Stackoverflow Survey](https://survey.stackoverflow.co/2023/#technology). | ||
|
||
Therefore we propose that a using more popular and widely used language such as TypeScript and a more popular and widely used framework such as React would significantly lower the barrier to entry to casual contributors to the Vela UI. | ||
|
||
<!-- We understand that Elm was originally for this codebase because... _please contribute this information_. --> | ||
|
||
**Please briefly answer the following questions:** | ||
|
||
1. Why is this required? | ||
|
||
<!-- Answer here --> | ||
|
||
Not required, however it should increase the number of contributors and contributions to the UI. | ||
|
||
2. If this is a redesign or refactor, what issues exist in the current implementation? | ||
|
||
<!-- Answer here --> | ||
|
||
- It is difficult to contribute to the UI, it is unlikely people who are interested in contributing know anything about Elm | ||
- In order to contribute it is likely the developer would need to learn a new language, which would put people off from contributing | ||
- The main project files are 5,000 - 7,000 lines of code each, which makes it difficult to find / figure out how things are tied together | ||
- ELM does not lend itself to code usability resulting in a copy / paste / rename / repurpose coding style. | ||
- There hasn't been any active development in the ELM framework for nearly 4 years, there are [many pull requests that have been sitting for 3+ years](https://github.com/elm/elm-lang.org/pulls?q=is%3Apr+is%3Aopen+sort%3Acreated-asc) | ||
|
||
3. Are there any other workarounds, and if so, what are the drawbacks? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are a couple different ways to look at increasing contributions. One could be the refactoring the codebase vs. rewriting in a different language. Is there a way to quantify the "increase" if a refactor is completed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Numerical considerations:
Less analytical considerations:
|
||
<!-- Answer here --> | ||
|
||
Alternatives to a redesign and rewrite could be... | ||
|
||
1. maintaining the existing Elm codebase | ||
2. creating and maintaining an extensive Elm-centric guide on maintenance activities, and new feature creation activities | ||
|
||
In general, while possible, this may not necessarily increase the overall casual contributions. | ||
|
||
However, with a redesign and rewrite... | ||
|
||
With a focus on React and other widely popular and community accessible tooling, we think we can increase casual contribution. | ||
|
||
This new rewrite will have: | ||
|
||
- common React patterns | ||
- common SPA patterns | ||
- reusable components | ||
- baseline guidance for creating new pages, widgets & components | ||
- baseline guidance for introducing new dependencies | ||
|
||
4. Are there any related issues? Please provide them below if any exist. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this leads us to a larger discussion around criteria / indicators that would cause components of the codebase to be rewritten. What criteria do we apply to Tech Debt to make decisions around prioritizing a rewrite? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not necessarily looking for Zac and Ryan to answer this question but it gets to the heart of all the areas of Vela... what criteria should we use to prioritizing rewriting any component? I like this question because it removes the immediacy of this proposal and allows us to take a better, unbiased look at the various factors. I'd love for everyone to reply to this section... |
||
|
||
As part of our initial exploration, we noticed a few discrepancies with the Swagger/OpenAPI spec. We will contribute to help address those issues. | ||
|
||
|
||
## Design | ||
|
||
<!-- | ||
This section is intended to explain the solution design for the proposal. | ||
|
||
NOTE: If there are no current plans for a solution, please leave this section blank. | ||
--> | ||
|
||
We have a suggested set of technologies to be considered for this redesign and rewrite, with justifications: | ||
|
||
- [TypeScript](https://www.typescriptlang.org/) | ||
- [popular](https://survey.stackoverflow.co/2023/#programming-scripting-and-markup-languages), widely used, industry standard language with well known tooling | ||
- enables type safety for better reliability and better developer experience | ||
- [React](https://react.dev/) | ||
- [popular](https://survey.stackoverflow.co/2023/#technology), widely used | ||
- enables common component patterns | ||
- can leverage popular react libraries | ||
- most _app_ developers have used react or a framework resembling react (vue, svelte) | ||
- [TailwindCSS](https://tailwindcss.com/) | ||
- [popular](https://2022.stateofcss.com/en-US/css-frameworks/), utility first centric "framework" for styles | ||
- easily portable to another framework in the future if necessary | ||
- easy to isolate styles for particular components | ||
- easy to create ad-hoc or situational tweaks | ||
- co-locates component markup/structure and style, reducing cross referencing and hunting | ||
- [Radix UI](https://www.radix-ui.com/) | ||
- Radix UI is a "headless" that provides accessibility features and enhanced experiences | ||
- [React Query](https://tanstack.com/query/latest/docs/react/overview) | ||
- [popular](https://www.npmjs.com/package/@tanstack/react-query), a data-fetching and state management library for React applications that simplifies fetching, caching, and updating data, and increases user experience by streamlining loading ui and increases developer experience by providing a consistent api for loading data and maintaining transient state | ||
- [Vite](https://vitejs.dev/) | ||
- [popular](https://www.npmjs.com/package/vite), [increasingly used](https://2022.stateofjs.com/en-US/libraries/), alternative to the now effectively deprecated [create react app](https://create-react-app.dev/) | ||
- enables an out of box single-page-app solution | ||
- enables better developer experience with fast refresh and fast build times | ||
- still extensible should special needs arise | ||
- [Vitest](https://vitest.dev/) | ||
- [popular](https://www.npmjs.com/package/vitest), a testing library with strong integration with Vite, and familiar to those with Jest flavored unit testing experience | ||
- works well in conjunction with [react-testing-library](https://testing-library.com/docs/react-testing-library/intro/) | ||
- [Cypress](https://www.cypress.io/) | ||
- re-using the existing cypress tests as a foundation to reach and maintain coverage | ||
- [Prettier](https://prettier.io/) | ||
- `gofmt` alike, automatic formatting without debate | ||
- [eslint](https://eslint.org/) | ||
- `golangci-lint` alike, automatic linting, recommending best coding practices | ||
|
||
<details> | ||
<summary><b>Why react?</b></summary> | ||
|
||
React has been a staple frontend "framework" in 2023. It has wide adoption and has a massive practitioner base. In the [2023 Stackoverflow Survey](https://survey.stackoverflow.co/2023/#section-admired-and-desired-web-frameworks-and-technologies), it continues to earn top a top position in desirability and admiration. | ||
|
||
We think that React, among others "frameworks" like Angular, Vue or Svelte fulfills our desire to increase casual contributors in this codebase. The principle reason is the sheer momentum that React has. | ||
|
||
React, like most frontend tooling, will enter its halcyon days and the industry will move to new tooling. There is no such thing as future-proof. However, in an effort to be future-resistant we are specifically making choices that will be easy to change around build tooling. We are not entirely leveraging the React core team's ecosystem (Next, CRA, Jest), and instead using a broader community ecosystem (Vite). | ||
|
||
</details> | ||
|
||
<details> | ||
<summary><b>Can we achieve the same and better results than Elm with this new approach?</b></summary> | ||
|
||
Elm offered a bundled developer experience for webapps. The React-centric ecosystem is much more fragmented, but it lets you pick from a variety of options that are easier to plug in and to swap as needs change. | ||
|
||
</details> | ||
|
||
We have considered these functional changes: | ||
|
||
- new/updated endpoints or url paths | ||
- re-evaluating the `/-/secrets` path | ||
- re-evaluating paths that shadow potentially valid Organization, Repository, or Secret names | ||
- new/updated configuration variables (environment, flags, files, etc.) | ||
- environment variables for features (e.g. default pull request settings, auto-linkifying in logs, etc) | ||
|
||
From an overall perspective, the goal is to: | ||
|
||
- Replicate the current user interface (look'n'feel; design) in the technology stack listed above as closely as possible | ||
- Replicate the current functionality in the technology stack listed above as closely as possible | ||
|
||
Consider this redesign and rewrite a `1-to-1.3̅3̅3̅` port rather than a `1-to-1` port. | ||
|
||
Further enhancements can and will be made after. | ||
|
||
## Implementation | ||
|
||
<!-- | ||
This section is intended to explain how the solution will be implemented for the proposal. | ||
|
||
NOTE: If there are no current plans for implementation, please leave this section blank. | ||
--> | ||
|
||
- Since the UI is a different repo and there are no co-dependency's the new UI should be a drop-in replacement and in theory should be able to run in parallel to the existing version if necessary. | ||
|
||
**Please briefly answer the following questions:** | ||
|
||
1. Is this something you plan to implement yourself? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would encourage you to provide more details on where you see Target engaging. While you might run with the majority of the rewrite, I still think this will take a lot of time/commitment from Target to help identify what feature-parity is, monitor milestones, engage Accessibility experts, and conduct the validation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I hope the Cargill teams responds to this comment as well.
After that, it is TypeScript & React code. The community is broad and could contribute as they need. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Has anyone looked at the current cypress tests? Are they extensive or should they be reviewed/updated?
Does Cargill have an Accessibility team? I know Target does. I don't know enough of the history of Vela to know if the current implementation went through a review but I strongly believe that any rewrite or mass-revision shoudl include this review (so we don't end up rewriting the rewrite). |
||
|
||
Yes. There is a small team working on this (see below; hackathon activity). | ||
|
||
2. What's the estimated time to completion? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This proposal was completed prior to the hackathon. Post-hackathon, can you provide more details and refine what it would take to complete this, how many people are committed, and their timeline to completion? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Neither of us are working on this full-time. I worked on this for fun because I use it for work regularly and I think it's a great tool. I want to add features. I want it to be approachable by more people. I want more people to use it. I would hope we are not replacing two primary contributors with an alternative set of two primary contributors. It's not a good exchange. It's not our intent. Hopefully this makes it easier for other contributors on the existing teams and in the community to join in. In our hackathon ui repo, we have an overview in the README that shows a majority outstanding tasks. We can add more detail. We have not updated this lately due to some internal feedback on this topic. Once we can align this proposal better, we can estimate and allocate time better. James mentioned 30-60 days in the document. We will refine that estimate when the time comes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I love the enthusiasm of individuals wanting to contribute. Naturally, when we hear that individuals are interested in rewriting "this for fun" makes us a little concerned about the overall commitment. This is a full-time job for us and our talents are spread across the entire codebase - we're not structured where we have two people working on the front-end and we'd just need to retrain those individuals on React. This is a whole-team effort for us - which is part of why we're taking this decision very seriously. |
||
|
||
30-60 days | ||
|
||
1. During the 2023 Summer Vela Hackathon, we explored this space and within roughly 62 hours we have ported a majority of the existing look'n'feel and functionality | ||
2. While a majority of read-centric screens exist, there are write-centric screens that need implementation | ||
3. There is still plenty of polishing, documentation and cleanup activities thereafter | ||
|
||
**Gists, issues, pull requests, etc** | ||
|
||
Please refer to the ongoing work in [`go-vela/ui-hackathon`](https://github.com/go-vela/ui-hackathon) repo for more details. | ||
|
||
## Questions | ||
|
||
**Please list any questions you may have:** | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you address "maintenance" after this would be implemented? Who is responsible for maintaining all the dependencies? What commitments should be made for breaking changes? Security updates are an important factor at Target that require 30 or 90 day updates - so would those all fall to Target? It would be nice to see more details around who is responsible after everything is released. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like all the maintenance would fall to Target. There are a couple ways to look at Elm but without any updates, there aren't any we need to apply - which has eliminated our maintenance effort. Reviewing the React release cycles, it looks like there's a major release once a year with a couple updates in the following months. Depending how our Security team classifies these, each update would require us to take action on either a 30 day or 90 day timeline. From experience, the 30 day cycles are very rough on our team becaus we need to drop everything we're doing and retool for the update. Enough of those back-to-back (like React had in 2022) and we fall behind on feature development because the last 6 months are spent just updating React. Breaking changes (like the ones called out in 18.0.0) extend our testing cycles. At at time when we're trying to be more predictable in our rollouts (1.5-2 months), something like 18.0.0 provides no value but takes up a lot of time. https://github.com/facebook/react/releases Because Elm doesn't require any maintenance today, React will force us to conduct maintenance. While this isn't monthly, it still appear to be disruptive to our schedules. |
||
- Can you share some additional details about how Elm was picked as the initial language/framework option for this codebase? We'd like to include this history and the pick's original goals as part of the proposal background. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been considering the benefits vs. costs of this proposal and are finding it difficult to quantify the potential for more commits. This is a lot of work to consider without having a better understanding for the benefit. As a reference point, we don't receive this feedback inside Target so we don't have experience with individuals telling us they don't know Elm and won't learn it but they'd contribute if it was written in a different language. Can you better define the "increase".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our organization today, we can estimate broadly about 1500 React flavored repositories and about 700 historical contributors to those repositories (probably around ~200 active contributors today). When we search for
elm
related repositories, we find none. The closest equivalent might be couple elixir with phoenix repositories. In addition to the surveys, we think this helps paint a picture representative of the ecosystem and industry today.Another metric is comparing the go-vela repositories among themselves (using this tool for stats):
⛳️: API only requires the last 1 year of commits in summary
I think these stats show that all the repos are healthily developed. I think some more interesting analysis could be done for the
Contributors
column - removing duplicates, looking at the long tail vs the top x contributors, etc.Unrelated to stats, Zac and I have wanted to contribute a few minor UI tweaks in the past. We looked into the Elm source and had a guess as what and how to change pieces, but it's certainly non-trivial for us. We work day-to-day in TypeScript and React. We agree this is familarity bias with our tech preferred stack. This is also how we build applications for internal and external customers too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The volume of developers doesn't necessarily indicate the volume of contributors to our project. They may indicate the "potential" number of contributors who are already familiar with the language. These potential contributors would still need to learn the project structure and familiarize themselves with the codebase. If these individuals are willing to go through the effort to do that, would they be just as willing to learn Elm?
Target has a variety of languages but we don't see trends of individuals on one team raising PRs on another team's repos. This would be the scenario your proposing. Sounds like Zac and Ryan have available time and interest. Do you have any other stats on this at Cargill? I'm trying to understand the actual increase in contributors we'd expect via this rewrite. At Target, we would still assume zero contributions from other teams and then be surprised/delighted if any came in.
Related to your individual story with "minor UI tweaks", did you seek any help internally or in the committer's channel or did you stop when you couldn't figure out Elm? The question doesn't intend to insult - we're trying to understand the volume of new contributions from individuals who walked away when they weren't willing or couldn't figure out Elm. This thread has the chance to focus on the theoretical benefits but fall apart in the reality. I acknowledge, this isn't an easy thing to measure and we don't have these stats at Target so curious if you have any.