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.

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

Format and move code in VCExpr project #738

Closed
wants to merge 3 commits into from
Closed

Format and move code in VCExpr project #738

wants to merge 3 commits into from

Conversation

keyboardDrummer
Copy link
Collaborator

@keyboardDrummer keyboardDrummer commented May 30, 2023 •
edited
Loading

Move classes from TypeErasure.cs into separate files. Format code in VCExpr project

@keyboardDrummer keyboardDrummer requested a review from atomb May 30, 2023 12:56
@keyboardDrummer keyboardDrummer marked this pull request as ready for review June 1, 2023 10:26
@atomb
Copy link
Collaborator

atomb commented Jun 1, 2023

It looks like one of the tests is failing. Any idea what might be behind that?

@atomb
Copy link
Collaborator

atomb commented Jun 1, 2023

I've noticed the same test failing on one of my PRs. It could be that it's just too non-deterministic. I'll look into whether that could be improved.

@atomb
Copy link
Collaborator

atomb commented Jun 2, 2023

I've made that test more deterministic as part of #741.

@keyboardDrummer keyboardDrummer enabled auto-merge (squash) June 5, 2023 11:34
@shazqadeer
Copy link
Contributor

shazqadeer commented Jun 5, 2023 •
edited
Loading

Since there wasn't much description in the PR summary, I took a quick look at the file changes and spotted the following:

  • location of open parenthesis for a class implementation has changed
  • renaming of MutatingVCExprVisitor to MutatingVcExprVisitor

Is the reformatting being done using some automated tool? Is the plan to make this style consistent throughout the code base eventually? Please add a few comments to the PR summary to let the reviewers know what you are thinking about.

I didn't spot any new files so presumably the splitting of classes in TypeErasure.cs is not done yet.

auto-merge was automatically disabled August 8, 2024 14:49

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atomb atomb Awaiting requested review from atomb

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
@keyboardDrummer @atomb @shazqadeer