frozenfrank
commented
5 months ago @19mdavenport
I agree that having days committed being independent of significant commits is fair, but that might be a good question for Dr. Wilkerson next Tuesday.
Great, let's confirm with Dr. Wilkerson on Tuesday, but confirming that detail shouldn't hold up this PR. That assumption was already in place before this, I just took this opportunity to point it out.
Might we ever want to store both the total number of commits and the number of significant commits together?
That were we start trying to look into a crystal ball to divine the future. I'm not sure. There are a bajillion different variations of the data we may care about, but I'm not sure which of them we care about yet.
This PR is adjusting code in a middle layer of the commit verification system. At the lowest layer, it is purely generating a list of the number of changes in each commit, but not asserting anything about how many of them are significant. At this layer, we are analyzing that list to count the ones that meet our threshold and report it as the number of significant commits. This is also the layer where we generate the error strings based on the crafted conditions we assert on the students.
The layer above that doesn't even really use the data we report. It mostly only pays attention to the CommitVerificationStatus.verified flag, and the rest of the fields are there for debug purposes. This object isn't saved in the database, but only used in memory to transfer data between layers of the application.
If we need data somewhere else, we'll get it there, but I'm not sure it's worth a lot of effort to expose all the data to some interface we haven't yet defined.
The previous code counted only the insertions in each commit. Per the definition we agreed on, this changes it to count both insertions and deletions. This issue will undercount the changes and will affect grading by effectively setting the line change threshold higher.
Additionally, the internal methods were previously reporting the total commits encountered instead of only the substantial commits. This issue does not affect grading.
This does raise a question about definitions. Currently, commits with low change counts still count towards the unique days with commits, even though they do not count towards the substantial commit threshold. This seems fair as it allows a student to do a little setup work on one day, and then do the bulk of the work on a few days. The student still had commits on multiple days even if the substantial commits were all on separate days. Is that legit?