DASSL / Gradebook

Open-source product to provide a practical means for instructors to record student attendance and assessment
Other
8 stars 4 forks source link

Address inconsistencies between schemas (M2) #78

Closed afig closed 5 years ago

afig commented 5 years ago

This PR addresses inconsistencies that existed between the conceptual/ER relational, and physical schemas in Gradebook. This was addressed by working off the schemas created in the CS305-Alpha fork and making changes (or more specifically, undoing changes) to make it contain only the features that were already implemented in DASSL's Gradebook.

The following changes were made to the conceptual schema to address inconsistencies:

A copy of the previous conceptual schema is available in docs/M1ConceptualSchema.png. This file may be removed before this PR is merged, depending on discussion. (The file will still be available through the repository's history.)

A full Logical schema (docs/M2LogicalSchema.docx) was added with descriptions for each table, as opposed to the previous logical schema which only contained a logical schema that was not fully consistent with either the old conceptual or physical schemas.

The changes contained by and description of this PR originally came from PR #77. However, due that PR's large size, many changes originally in the PR have been set aside for later review, discussion, and/or re-implementation.

Closes #41

MatthewChastain commented 5 years ago

I believe that the changes made adequately address the issue of inconsistencies between the conceptual and logical schemas.

Is the plan to remove the material in the logical schema regarding changes made to the existing schema? There doesn't seem to be a purpose to having that information in the document if this is going to be overwriting the existing schema. I also don't think that the existing conceptual schema need be included in the PR when it comes time to merge.

Further, what are the team's thoughts on relationship names in the conceptual schema? Is that something that is planned for another PR at some point?

afig commented 5 years ago

Thank you for the review @MatthewChastain (and @jrm86).

Upon further review, it seems like an incorrect version of the logical schema was pushed to the repo. The new version (pushed a few minutes ago) does not contain the listing of changes made to the existing schema, which addresses your first question. (it also has updated author information, and some leftovers from Team Alpha's work removed). I agree that a written listing is not necessary, as the information is maintained in this PR.

I am okay with removing M1's schemas before the final merge.

Also, good point about relationship names, I hadn't considered them, but they could be useful. We can add them now, or later when/if we decide to make the diagram follow the ER syntax more closely.