这是indexloc提供的服务，不要输入任何密码
Skip to content

Rename TopologicGraph to WorkspaceGraph #3411

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 23, 2023
Merged

Rename TopologicGraph to WorkspaceGraph #3411

merged 4 commits into from
Jan 23, 2023

Conversation

mehulkar
Copy link
Contributor

@mehulkar mehulkar commented Jan 20, 2023 •
edited
Loading

TopologicGraph in the engine sounds like it's for tasks, since the engine is for executing tasks, but it's really the workspace graph.

@vercel
Copy link

vercel bot commented Jan 20, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
examples-svelte-web 🔄 Building (Inspect) Jan 23, 2023 at 6:57PM (UTC)
9 Ignored Deployments
Name Status Preview Comments Updated
examples-basic-web ⬜️ Ignored (Inspect) Jan 23, 2023 at 6:57PM (UTC)
examples-cra-web ⬜️ Ignored (Inspect) Jan 23, 2023 at 6:57PM (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Jan 23, 2023 at 6:57PM (UTC)
examples-kitchensink-blog ⬜️ Ignored (Inspect) Jan 23, 2023 at 6:57PM (UTC)
examples-native-web ⬜️ Ignored (Inspect) Jan 23, 2023 at 6:57PM (UTC)
examples-nonmonorepo ⬜️ Ignored (Inspect) Jan 23, 2023 at 6:57PM (UTC)
examples-tailwind-web ⬜️ Ignored (Inspect) Jan 23, 2023 at 6:57PM (UTC)
turbo-site ⬜️ Ignored (Inspect) Visit Preview Jan 23, 2023 at 6:57PM (UTC)
turbo-vite-web ⬜️ Ignored (Inspect) Jan 23, 2023 at 6:57PM (UTC)

@mehulkar
Rename TopologicGraph to be more clear
461ad21 
TopologicGraph in the engine sounds like it's for tasks, since
the engine is for executing tasks, but it's really the workspace graph.
@mehulkar mehulkar changed the title Rename some things to be more clear @mehulkar Rename TopologicGraph to be more clear Jan 21, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 21, 2023 •
edited
Loading

🟢 CI successful 🟢

Thanks

@mehulkar mehulkar changed the title @mehulkar Rename TopologicGraph to be more clear Rename TopologicGraph to be more clear Jan 21, 2023
@mehulkar mehulkar marked this pull request as ready for review January 21, 2023 00:20
@mehulkar mehulkar requested a review from a team as a code owner January 21, 2023 00:20
@mehulkar mehulkar mentioned this pull request Jan 23, 2023
Merged
2 tasks
@NicholasLYang
Copy link
Contributor

Just to be clear, topological in this context means topologically sorted? Seems like there's other variables in this context using that term and I'm not really sure what it implies. Otherwise LGTM

@mehulkar
Copy link
Contributor Author

mehulkar commented Jan 23, 2023 •
edited
Loading

Just to be clear, topological in this context means topologically sorted?

Not really, it's a graph data structure about which workspaces are connected to which. It's built up by looking at dependencies between workspaces: https://github.com/vercel/turbo/blob/671f89dcbdb85567b94c13609240c5a158bc6f99/cli/internal/context/context.go#L162

https://github.com/vercel/turbo/blob/671f89dcbdb85567b94c13609240c5a158bc6f99/cli/internal/context/context.go#L146

@mehulkar mehulkar requested review from chris-olszewski and removed request for nathanhammond January 23, 2023 17:23
@mehulkar mehulkar changed the title Rename TopologicGraph to be more clear Rename TopologicGraph to WorkspaceGraph Jan 23, 2023
@NicholasLYang
Copy link
Contributor

Hmm yeah maybe this is me overthinking this but topological is a really general word. I'd be in favor of removing it entirely unless we're talking about topological sorting.

tknickman
tknickman approved these changes Jan 23, 2023
View reviewed changes
Copy link
Member

@tknickman tknickman left a comment

Choose a reason for hiding this comment

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

Can you also rename some of the functions in context.go (like populateTopologicGraphForPackageJSON for example) - I think that's the last place we refer to TopologicalGraph

@mehulkar mehulkar enabled auto-merge (squash) January 23, 2023 18:56
@mehulkar
Copy link
Contributor Author

@tknickman great call, updated!

@mehulkar mehulkar merged commit 9142109 into main Jan 23, 2023
@mehulkar mehulkar deleted the mk/renames branch January 23, 2023 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tknickman tknickman tknickman approved these changes

@NicholasLYang NicholasLYang NicholasLYang approved these changes

@chris-olszewski chris-olszewski Awaiting requested review from chris-olszewski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mehulkar @NicholasLYang @tknickman