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

Better handling for DLC with missing readme.txt #3935

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

HebaruSan
Copy link
Member

Problem

@JonnyOThan reported the following error with the current dev build:

Scanning for DLCs and manually installed modules...
MakingHistory-DLC missing required field version
Repository update failed!

Cause

The DLC's folder was present, but its readme.txt file was absent. This caused its CkanModule.version property to be missing, which isn't allowed.

Originally this would have defaulted to new UnmanagedModuleVersion(null), but this was lost in #3904's refactoring.

Changes

Now we once again default to new UnmanagedModuleVersion(null), as per previous releases. A new test exercises this case to make sure Registry.SetDLCs doesn't start throwing again.

I'll probably self-review this since it's small and a regression.

@HebaruSan HebaruSan added the Bug Something is not working as intended label Dec 4, 2023
@HebaruSan HebaruSan added Easy This is easy to fix Core (ckan.dll) Issues affecting the core part of CKAN Tests Issues affecting the internal tests Registry Issues affecting the registry labels Dec 4, 2023
@HebaruSan HebaruSan merged commit 2c137f6 into KSP-CKAN:master Dec 4, 2023
8 checks passed
@HebaruSan HebaruSan deleted the fix/missing-dlc-version branch December 4, 2023 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is not working as intended Core (ckan.dll) Issues affecting the core part of CKAN Easy This is easy to fix Registry Issues affecting the registry Tests Issues affecting the internal tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant