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

Break up dafny ast #2694

Closed

Conversation

keyboardDrummer
Copy link
Member

Break up DafntyAst.cs into smaller chunks that Rider can process well.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

Copy link
Member

@cpitclaudel cpitclaudel left a comment

Choose a reason for hiding this comment

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

Can we make sure to keep history in this PR? To do that we'll have to change our "squash-only" policy and do each move as a merge commit, but the alternative (merging this as-is) will break git -L, which I use multiple times a day on Dafny's codebase.

@keyboardDrummer
Copy link
Member Author

keyboardDrummer commented Sep 5, 2022

Can we make sure to keep history in this PR? To do that we'll have to change our "squash-only" policy and do each move as a merge commit, but the alternative (merging this as-is) will break git -L, which I use multiple times a day on Dafny's codebase.

From what I see on stack overflow, retaining the history on a change that splits off part of the code in a file requires a particular sequence of steps. Could you document these in the contributing documentation? I see we don't have a CONTRIBUTING.md but maybe this is a good reason to create one.

but the alternative (merging this as-is) will break git -L, which I use multiple times a day on Dafny's codebase.

Would it be possible to write a script that does git -L in a codebase where the code moves are not tracked? Something like:

  1. Find the commit where the line of code was introduced.
  2. Check out the previous commit, grep the codebase for the same line
  3. If you find a hit, go back to 1

I'm asking because I foresee a lot of code moves in Dafny's future, some large but also many small, and I do not think there's an effective way of tracking these through git so git log -L will work for them. The technique in the stackoverflow post I linked above works for code that is moved to a fresh file, but not for code that is moved between existing files.

@cpitclaudel
Copy link
Member

Could you document these in the contributing documentation? I see we don't have a CONTRIBUTING.md but maybe this is a good reason to create one.

Sure, I can do that.

Would it be possible to write a script that does git -L in a codebase where the code moves are not tracked?

Perhaps, though it sounds tricky, due to e.g. code duplication

@keyboardDrummer
Copy link
Member Author

Would it be possible to write a script that does git -L in a codebase where the code moves are not tracked?

Perhaps, though it sounds tricky, due to e.g. code duplication

Have you tried using git log -S <string> ?

-S
Look for differences that change the number of occurrences of the specified string (i.e. addition/deletion) in a file. Intended for the scripter's use.
It is useful when you're looking for an exact block of code (like a struct), and want to know the history of that block since it first came into being: use the feature iteratively to feed the interesting block in the preimage back into -S, and keep going until you get the very first version of the block.

I tried it on several pieces of the code moved in this PR and it detects the last commit these code pieces were changed before this move, at which point you could do git log -L again

@cpitclaudel
Copy link
Member

Yes, -S helps a lot when -L fails.

auto-merge was automatically disabled September 30, 2022 13:17

Pull request was closed

@keyboardDrummer
Copy link
Member Author

Superseded by #2705

@keyboardDrummer keyboardDrummer deleted the breakUpDafnyAst branch September 30, 2022 13:17
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

2 participants