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

Update KerbalAircraftExpansion.netkan #2863

Closed
wants to merge 3 commits into from
Closed

Conversation

politas
Copy link
Member

@politas politas commented Dec 24, 2015

Adding Epoch to fix new version sorting and install example craft now that we can do that. Other exclusions have been removed from download.

Adding Epoch to fix new version sorting and install example craft now that we can do that. Other exclusions have been removed from download.
@politas
Copy link
Member Author

politas commented Dec 24, 2015

See also KSP-CKAN/CKAN-meta#911

@mheguy
Copy link
Contributor

mheguy commented Dec 24, 2015

From what I see, we're already offering the correct version KAX_v2.6.1.0. I don't see a need for an epoch bump.

@politas
Copy link
Member Author

politas commented Dec 24, 2015

The Max KSP is listing as 1.0.4, though.

@mheguy
Copy link
Contributor

mheguy commented Dec 24, 2015

Oh I see what you're talking about: we have a 1.0.5-compatible version but the "Max KSP Version" column incorrectly displays the compatibility from a previous version. Basically this is KSP-CKAN/CKAN#1327 emerging again. We should probably disable that column until the issue is resolved. I'll merge this and KSP-CKAN/CKAN-meta#911 as a bandaid fix (once we sort out the technical errors in the PRs).

@politas
Copy link
Member Author

politas commented Dec 24, 2015

Yep. I've been steadily working through applying bandaids for that issue.

},
{
"find": "Example Craft",
"install_to": "Ships/SPH"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not work as we cannot create Ships/SPH/Example Craft/. .craft files must be installed directly to Ships/SPH/. Thus we filter the Example Craft.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. I thought it would just install the files from that directory into Ships/SPH.
I guess we'll need to use "find_matches_files" : "true", and make it a find_regexp "Example.Craft.*craft". I'll test it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Damn. I can't test it. My netkan keeps getting authentication errors.

@mheguy
Copy link
Contributor

mheguy commented Dec 24, 2015

Instead of bandaid epoch bumps that make version numbers harder to discern for users and contributors alike, I'd much rather fix the problem with the column itself, disabling it in the interim if necessary. Realizing now what all your previous PRs were doing I wish I had rejected them and disabled the column from the start.

@politas
Copy link
Member Author

politas commented Dec 24, 2015

They aren't all for that reason.

@politas
Copy link
Member Author

politas commented Dec 24, 2015

Well, that find_regexp works partially. It only installs one of the Example Craft, but correctly puts the craft file into the SPH folder. Looks like we have to do separate finds for each craft. I'm surprised that setting find_matches_files true doesn't allow multiple files with a find_regexp.

@politas
Copy link
Member Author

politas commented Dec 25, 2015

I do think that an epoch bump is called for when a modder changes the version string system they use, which keptin has certainly done in this case.

Remove Example Craft install
@politas
Copy link
Member Author

politas commented Dec 25, 2015

Ok, I've reverted the Example Craft installation and will make that a separate change (if I can get it working in a sane way)

@mheguy
Copy link
Contributor

mheguy commented Jan 6, 2016

Spec_ver increase limits CKAN version compatibility unnecessarily and the added comment is inaccurate. For simplicity, created #2907 which closes this.

@politas politas closed this Jan 6, 2016
@politas politas deleted the patch-3 branch January 6, 2016 21:32
@politas
Copy link
Member Author

politas commented Jan 6, 2016

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants