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

Fix crash on startup (fixes #2134) #2138

Merged
merged 2 commits into from
Oct 14, 2017

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Oct 14, 2017

If a mod isn't in the registry, return null instead of raising an exception when looking for its max KSP version

When I tested this code change, I had a very recent registry, so the code only ran with identifiers that were in the dictionary. Users updating to 1.22.4 encountered newly added mods, which caused an exception as reported in #2134. Checking ContainsKey first will avoid this, and returning null signals that the mod is not in the registry, which will cause the column to default to max version of the newly added mod object.

If a mod isn't in the registry, return null instead of raising an
exception when looking for its max KSP version
@Olympic1 Olympic1 added the Registry Issues affecting the registry label Oct 14, 2017
@HebaruSan
Copy link
Member Author

Based on comment in #2137, @Olympic1 intended to merge this by approving it.
Attempting to merge...

@HebaruSan HebaruSan merged commit 20e4e91 into KSP-CKAN:master Oct 14, 2017
@politas
Copy link
Member

politas commented Oct 15, 2017

@HebaruSan & @Olympic1, can I just point you to https://github.com/KSP-CKAN/CKAN/wiki/Releasing-a-new-CKAN-client which has some points regarding merging PRs to the codebase? We really want to keep the Changelog up to date.

@HebaruSan HebaruSan deleted the fix/latest-ksp-exception branch October 15, 2017 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Registry Issues affecting the registry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants