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

Remove Context trait from chalk-engine #611

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

jackh726
Copy link
Member

This is unneeded now that chalk-engine knows about chalk-solve. The only reason we might want this is for abstraction, but I don't think that really gets us very far here.

There's more that could be done here, for sure. But this has been sitting locally for a while and figured it's a good enough place as any to PR.

@nikomatsakis
Copy link
Contributor

This looks really good. I kinda' miss the old traits but I think concrete code will ultimately be much easier for folks to read.

@nikomatsakis
Copy link
Contributor

r=me to land @jackh726

@jackh726
Copy link
Member Author

I kinda' miss the old traits but I think concrete code will ultimately be much easier for folks to read.

I definitely understand this sentiment. It's sort of nice to have an abstraction. But I think ultimately it isn't necessary anymore and makes it harder to understand.

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Sep 29, 2020

📌 Commit 0cdc6c7 has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Sep 29, 2020

⌛ Testing commit 0cdc6c7 with merge 9325fb6...

@bors
Copy link
Collaborator

bors commented Sep 29, 2020

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 9325fb6 to master...

@bors bors merged commit 9325fb6 into rust-lang:master Sep 29, 2020
@jackh726 jackh726 deleted the chalk-engine-context branch September 29, 2020 23:43
detrumi added a commit to detrumi/chalk that referenced this pull request Oct 4, 2020
This broke a while back in rust-lang#611,
but only now broke the build because the hosted docs were updated.
bors added a commit that referenced this pull request Oct 4, 2020
Fix mdbook links after chalk-engine changes

This broke a while back in #611,
but only now broke the build because the hosted docs were updated.

CC `@jackh726`
@jackh726 jackh726 restored the chalk-engine-context branch December 15, 2020 19:40
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.

3 participants