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

Implement remainder of local Vcs: casting and get_task_id #8871

Closed
wants to merge 1 commit into from

Conversation

bgw
Copy link
Member

@bgw bgw commented Jul 30, 2024

Description

With these changes, local Vcs (introduced in #8780) should have a fully functional implementation.

These remaining methods were pretty straightforward.

After this, my focus will shift back towards the changes needed to tasks to allow us to make use of local Vcs.

New resolve_type_inner helper method

I merged the implementations of resolve_trait and resolve_value, since their implementations were 95% identical. I think this is an overall win, though the logic to prevent duplicate ValueType lookups (they're not expensive, but this code is also potentially very hot) is a bit messy.

Testing Instructions

cargo nextest r -p turbo-tasks -p turbo-tasks-memory

Copy link

vercel bot commented Jul 30, 2024

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

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 30, 2024 0:12am
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 30, 2024 0:12am
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 0:12am
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 0:12am
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 0:12am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 0:12am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 0:12am
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 0:12am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 0:12am
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Jul 30, 2024 0:12am

Copy link
Member Author

bgw commented Jul 30, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bgw and the rest of your teammates on Graphite Graphite

@bgw bgw changed the title Implement remainder of todos for local Vcs Implement remainder of local Vcs: casting and get_task_id Jul 30, 2024
Copy link
Contributor

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

✅ This change can build next-swc

@bgw bgw marked this pull request as ready for review July 30, 2024 00:15
@bgw bgw requested a review from a team as a code owner July 30, 2024 00:15
Copy link
Contributor

github-actions bot commented Jul 30, 2024

⚠️ CI failed ⚠️

The following steps have failed in CI:

  • Turbopack Rust tests (mac/win, non-blocking)

See workflow summary for details

@bgw
Copy link
Member Author

bgw commented Aug 2, 2024

Moved to the next.js repo: vercel/next.js#68474

@bgw bgw closed this Aug 2, 2024
@bgw bgw deleted the bgw/local-vc-casting branch August 4, 2024 19:50
bgw added a commit to vercel/next.js that referenced this pull request Aug 5, 2024
…#68474)

*This is a migrated PR. This was in the turbo repository before the
next.js merge.*
**Migrated From:** vercel/turborepo#8871

## Description

With these changes, local Vcs (introduced in #68469) should have a fully
functional implementation.

These remaining methods were pretty straightforward.

After this, my focus will shift back towards the changes needed to tasks
to allow us to make use of local Vcs.

## New `resolve_type_inner` helper method

I merged the implementations of `resolve_trait` and `resolve_value`,
since their implementations were 95% identical. I think this is an
overall win, though the logic to prevent duplicate `ValueType` lookups
(they're not expensive, but this code is also potentially very hot) is a
bit messy.

## Testing Instructions

```
cargo nextest r -p turbo-tasks -p turbo-tasks-memory
```
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 14, 2024
…#68474)

*This is a migrated PR. This was in the turbo repository before the
next.js merge.*
**Migrated From:** vercel/turborepo#8871

## Description

With these changes, local Vcs (introduced in #68469) should have a fully
functional implementation.

These remaining methods were pretty straightforward.

After this, my focus will shift back towards the changes needed to tasks
to allow us to make use of local Vcs.

## New `resolve_type_inner` helper method

I merged the implementations of `resolve_trait` and `resolve_value`,
since their implementations were 95% identical. I think this is an
overall win, though the logic to prevent duplicate `ValueType` lookups
(they're not expensive, but this code is also potentially very hot) is a
bit messy.

## Testing Instructions

```
cargo nextest r -p turbo-tasks -p turbo-tasks-memory
```
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 15, 2024
…#68474)

*This is a migrated PR. This was in the turbo repository before the
next.js merge.*
**Migrated From:** vercel/turborepo#8871

## Description

With these changes, local Vcs (introduced in #68469) should have a fully
functional implementation.

These remaining methods were pretty straightforward.

After this, my focus will shift back towards the changes needed to tasks
to allow us to make use of local Vcs.

## New `resolve_type_inner` helper method

I merged the implementations of `resolve_trait` and `resolve_value`,
since their implementations were 95% identical. I think this is an
overall win, though the logic to prevent duplicate `ValueType` lookups
(they're not expensive, but this code is also potentially very hot) is a
bit messy.

## Testing Instructions

```
cargo nextest r -p turbo-tasks -p turbo-tasks-memory
```
ForsakenHarmony pushed a commit to vercel/next.js that referenced this pull request Aug 16, 2024
…#68474)

*This is a migrated PR. This was in the turbo repository before the
next.js merge.*
**Migrated From:** vercel/turborepo#8871

## Description

With these changes, local Vcs (introduced in #68469) should have a fully
functional implementation.

These remaining methods were pretty straightforward.

After this, my focus will shift back towards the changes needed to tasks
to allow us to make use of local Vcs.

## New `resolve_type_inner` helper method

I merged the implementations of `resolve_trait` and `resolve_value`,
since their implementations were 95% identical. I think this is an
overall win, though the logic to prevent duplicate `ValueType` lookups
(they're not expensive, but this code is also potentially very hot) is a
bit messy.

## Testing Instructions

```
cargo nextest r -p turbo-tasks -p turbo-tasks-memory
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant