Changes in License applyed #5
Conversation
MaxThomasHPI
commented
Feb 12, 2024
MaxThomasHPI
commented
- as discussed, there were problems/inconsistencies in the License object
- updated to the latest version
- documentation was aligned -> description attribute in the schema updated
- removed outdated uml diagrams
- please check!
- documentation aligned (description in the attributes) - added clarification about general license according to spdx and specific license
|
Thanks for working on these changes. I'll leave it to @Dome-GER and @cwillems to have a detailed look. Just two remarks ahead:
PS: Feel free to adjust the commit message [especially the second one] to provide a better overview of the changes. Here are some thoughts on writing good commit messages. |
The former yml file was just a translation of the json schema. Now the yml file describes the whole API. Information about pagination was missing as well as HTTP error messages. yml file had the wrong indention in arrays. Fixing this improved readability. There was a naming error calling the MOOChub schema "moochub_schema.json" instead of "moochub-schema.json". This caused problems in matching the correct files in the github repository.
|
Thanks, it's getting easier to compare the changes. Still, I have a few remarks:
|
|
have just recently found another error in my pull request, will take a look at these new comments afterward. speaking of |
There was a mistake that "instructor" was not an array. This was in contradiction to the defined schema.
Potentially, yes. However, it seems to depend on the specific schema version used; the newer ones do not include the |
There were some spaces in the honorific prefix missing in some descriptions and examples. This was a clear mistake and made reviewing changes unnecessary harder.
There was a problem that with generating the yml file by a script the quotation marks were sometimes missing in the yml. This caused a lot of trouble in reviewing changes in the file, since all missing quotation marks were treated as changes. The script was updated and the file shows quotation marks now where needed.
The affiliation in the instructor attribute was wrongly indented. It was part of the image attribute. By correcting the indention the affiliation is now part of the Person object in the instructor attribute as intended.
|
Just another remark: All URLs pointing to https://openhpi.azureedge.net are no longer valid, since we are stopped using this host to deliver assets. |
|
ok, is this relevant? Should only be an issue with the images, and they are just examples? Could also e something like: www.example.com/image.png? |
Well, it doesn't make the example easier to understand nor complete. Maybe it's just something for a dedicated ticket / PR? The smaller each ticket and the corresponding PR is, the easier (and faster!) they are to review 😉.
Yes, and they serve the purpose to illustrate with valid examples how an entry could look like (at least that's how I would argue). |
There were some problems with data types of attributes. Namely, some attributes can have a value of a defined type or can be 'null'. In yml this was represented as an array. As it is in the json. For consistency, the yml will now only show the data type in the type attribute and has a nullable attribute as before. There were still issues with quotation marks that showed changes where are none. In the older version, empty attributes contained the value 'null'. This will be the case also for the newer version. This way, no unnecessary tracking of changes occurs.
A few discrepancies to the old file were detected and they are only of cosmetic nature. These were fixed to allow for better comparability of real changes in the yml file.
Another alignment to fit the od style to avoid unnecessary change tracking.
The repeatFrequency contained the following attributes which should be on the same level as repeateFrequency. The mistake is corrected to fit the intended schema. This is also removing some validation errors.
The LearningItem object in the hasPart attribute had a mistake. The required attribute was on the wrong level. Correction of will make the attribute hasPart work as expected again. This also erases a validation error in the yml file.
There was a problem in the "Problem" part of the yml file. the required attribute as well as readOnly and x-examples were at the wrong level. Fixing this erases validation errors in the yml file and leads to fewer unwanted changes to be tracked. Also corrects a mistake that tags now is an array as intended.
The indention of the old file and the new version were different. This made it impossible to track changes.
The level the data object was stored in the JSON schema was wrong. Now the data is stored as intended at the same level as the links.
The There was a problem that the license attribute contained an array of arrays of License objects. Intended is to have the license attribute of the course storing an array of License objects.
The There was a problem that the educationalLevel attribute did not contain an array of EducationalLevel objects. Intended is to have the educationalLevel attribute of the course storing an array of EducationalLevel objects.
It seems like draft 7 has the |
I don’t mind, feel free to use the latest version. In that case, however, you should also check for potential changes. |
The example is now matching the identifier and is pointing at the respective spdx url. It complies to the rules set in the description now.
Before the change the link pointed at images on the azure server and are not valid any longer. The new links are working and point an existing image that is reachable.
The changes could not be reviewed because of too many changes. The referencing style is now again like in the former version. There still seems to be an advantage in outsourcing repeating parts in extra files.
Only minor changes were made. The functionality was not altered. The change is mainly for solving inconveniences, update examples, ...
