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

add proposal for rewrite of vela ui #831

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

JayCeeJr
Copy link
Contributor

add proposal for rewrite of vela ui

@JayCeeJr JayCeeJr requested a review from a team as a code owner June 27, 2023 17:24
@ryanmr
Copy link

ryanmr commented Jun 30, 2023

Hi all,

I've been working with @JayCeeJr and @zapplebee on this before and during our 2023 Summer Vela Hackathon time.

Here's the rendered text for easier reading.

I am very happy to make any changes or answer questions.

Thanks for having us.


<!-- Answer here -->

Not required, however it should increase the number of contributors and contributions to the UI.
Copy link
Contributor

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".

Copy link

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):

Commits⛳️ Issues Forks Contributors PRs
go-vela/worker 107 7 7 18 487
go-vela/cli 66 13 9 22 451
go-vela/ui 113 10 15 26 694
go-vela/server 195 21 26 32 889
go-vela/docs 52 28 32 40 341

⛳️: 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.

Copy link
Contributor

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.

- 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?
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Numerical considerations:

  • Number of contributors applies either way
  • Age of open tickets
  • Age of open proposals

Less analytical considerations:

  • Population: if there are fewer possible contributors, there will be fewer possible contributions
  • Quality of prototyping / experimenting
  • Ease of casual contributions
  • Better alignment to existing patterns in the wild

- 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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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...


**Please briefly answer the following questions:**

1. Is this something you plan to implement yourself?
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope the Cargill teams responds to this comment as well.

  • Feature parity
    • In my opinion, parity is replicating the existing functionality and aligning closely with the look'n'feel
    • Do cypress tests pass with the appropriate changes?
  • Monitor milestones
    • Cargill internal dogfooding prior to a GA-release
    • In a later section, there is more discussion on remaining and perhaps details on timelines
    • This feels like a low touch commitment; nobody wants a drawn out parallel implementation indefinitely
  • Engage Accessibility experts
    • I am not an accessibility expert, but where it was in place in the existing user interface, it is intended to be replicated, and where it was not implemented already, it will be added where possible
    • I appreciate any expertise and time given towards this; and Cargill has UX experts internally to tap too if required
  • Conduct the validation
    • re: dogfooding
    • I appreciate the diligience this could take. Hopefully an equvilent cypress test suite and dogfooding could accommodate part of the validation process
    • There are a dozen pages and few dozen notable page states that I think we could quickly cover

After that, it is TypeScript & React code. The community is broad and could contribute as they need.

Copy link
Contributor

@chrispdriscoll chrispdriscoll Jul 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do cypress tests pass with the appropriate changes?

Has anyone looked at the current cypress tests? Are they extensive or should they be reviewed/updated?

Engage Accessibility experts

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?
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither of us are working on this full-time

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.

## Questions

**Please list any questions you may have:**

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Can you address "maintenance" after this would be implemented?
    • With renovate and some documentation, maintenance tasks should be fairly low effort
  • Who is responsible for maintaining all the dependencies?
    • Who maintained this for Elm?
    • Related to the topic of contributors, I'd hope there's more than just a replacement of primary contributors. Meanwhile, renovate should certainly pickup a majority of changes
  • What commitments should be made for breaking changes?
    • What commitments exist today?
    • Breaking changes for user interface are different than breaking changes for api
    • AFAIK, this is pre 1.0 from some perspective
  • Security updates are an important factor at Target that require 30 or 90 day updates - so would those all fall to Target?
    • I would like to learn more about this. Have there been many major security incidents with this user interface in the past? Have regular patch versions with upgrades been rolled out from the open source repo? Excluding the nginx host and other server side bits?
    • The surface area does change from Elm to a more JS-centric ecosystem
    • Again, with renovate and the ease of updating dependencies, it should not be overwhelming regardless of which team or community members are engaging with it

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

@chrispdriscoll
Copy link
Contributor

I don't think this fits into a specific section but is referenced in a couple places - my personal thought is this will still require a lot of time and energy from Target. After just completing 12 months of rewrites to the security posture, I'm a bit exhausted with rewrites and would love to focus our energy on new capabilities. Scheduled Builds was driven by Cargill and I observed a lot of energy from Target for us to get through the security work and back into new development.

Perhaps my philosophical question is: is now the time to do this?
Rewrite exhaustion might be a good reason to postpone but the more direct question is: what criteria should we use to identify when a component of the tech stack should be rewritten?

@ryanmr
Copy link

ryanmr commented Jul 21, 2023

I don't think this fits into a specific section but is referenced in a couple places - my personal thought is this will still require a lot of time and energy from Target. After just completing 12 months of rewrites to the security posture, I'm a bit exhausted with rewrites and would love to focus our energy on new capabilities. Scheduled Builds was driven by Cargill and I observed a lot of energy from Target for us to get through the security work and back into new development.

So far this effort is primarily on the Cargill side. We can certainly do our due diligence in regards to churn, security, and rewrite fatigue. We hope that this change also opens the ui up for new development too, more readily even.

Perhaps my philosophical question is: is now the time to do this? Rewrite exhaustion might be a good reason to postpone but the more direct question is: what criteria should we use to identify when a component of the tech stack should be rewritten?

Rewrite exhaustion may be a good reason to defer this concept for now. There is definitely a balance between churn and new development. That said, as the folks on the recent vela call discussed, we can approach docs and onboarding materials first, or later, but these concepts are coupled either way.

@chrispdriscoll
Copy link
Contributor

Rewrite exhaustion may be a good reason to defer this concept for now.

The more I've looked into the maintenance side of React, the more I'd really like us to have more core functionality in place before we add another layer of maintenance. React updates will take away from our new feature development so let's be sure we keep a long lens on the costs and benefits of this switch.

@chrispdriscoll
Copy link
Contributor

On August 2, we met to discuss and layout a plan going forward. The following Action Items and timeline were agreed...

Next Steps
1. Refactor one page in Elm to common patterns
2. Compare with React rewrite
3. Add Comments/Documentation around Elm codebase
4. Identify tips/tricks on Elm plugins and how to setup machine for Elm development

Timeline: Target would like to focus on getting v0.21 and v0.22 released externally and internally before our Q4 Freeze (early Oct). We'd like to pick this up after the v0.22 release. (this has been updated on the roadmap and queued for November)
https://github.com/orgs/go-vela/projects/13

Parallel effort: Develop proposal for revisiting the Tech Stack

@ryanmr
Copy link

ryanmr commented Sep 26, 2023

Hi all,

Thanks for all your work on this thread. Please feel free to update the PR with any appropriate status you feel is necessary for organization, or feel free to close this while other work proceeds.

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

Successfully merging this pull request may close these issues.

4 participants