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

refactor: Upgrade svelvet #358

Merged
merged 19 commits into from
Oct 9, 2023
Merged

refactor: Upgrade svelvet #358

merged 19 commits into from
Oct 9, 2023

Conversation

avivash
Copy link
Member

@avivash avivash commented Oct 5, 2023

Description

  • Upgrade svelvet to v8(using new Anchor, Edge and Node components)
  • Maintain Node positions, so running workflow one after workflow two will no long cause the Nodes to jump up to the first row

Link to issue

#337

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • Refactor (non-breaking change that updates existing functionality)

Test plan (required)

  1. Run cargo run -- start -c config/settings.toml from homestar/examples/websocket-relay
  2. Run npm run dev from homestar/examples/websocket-relay/relay-app
  3. Visit http://localhost:5173/
  4. Run workflow one and expect the Nodes and edges to appear mostly as they did before(black edges with arrows and labels describing the associated tasks)
  5. Run workflow two and expect the grayscale Node to appear in the bottom row with an edge connected to its north side
  6. Refresh the page
  7. Run workflow two and expect the Nodes, Edges and labels to appear correctly in the bottom row
  8. Run workflow one and expect the Nodes that have already been executed to stay in the bottom row, but also have the blurred Node rendered in the top row with an Edge connected to it

Screenshots/Screencaps

https://www.loom.com/share/acc35d252e8b4d5a94a6f0d53aecfa91?sid=54cdb36b-1837-4082-b759-f8e80f36503e

@avivash avivash self-assigned this Oct 5, 2023
@avivash avivash requested a review from a team as a code owner October 5, 2023 22:03
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #358 (89645bb) into main (0eeb0bf) will decrease coverage by 0.66%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #358      +/-   ##
==========================================
- Coverage   72.83%   72.18%   -0.66%     
==========================================
  Files          69       69              
  Lines        6848     6848              
==========================================
- Hits         4988     4943      -45     
- Misses       1860     1905      +45     

see 3 files with indirect coverage changes

Copy link
Contributor

@bgins bgins left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks for taking this one on. 🙏

@avivash
Copy link
Member Author

avivash commented Oct 7, 2023

Looks good! Thanks for taking this one on. 🙏

No worries! Sorry it took about three days longer than I'd expected 😅 on the bright side, I'm feeling much more knowledgable about both homestar and svelvet now 👍🏼

@bgins bgins changed the title Upgrade svelvet refactor: Upgrade svelvet Oct 9, 2023
@zeeshanlakhani zeeshanlakhani merged commit 15de823 into main Oct 9, 2023
27 checks passed
@zeeshanlakhani zeeshanlakhani deleted the upgrade-svelvet branch October 9, 2023 23:15
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.

None yet

3 participants