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

Use Harmony bundled with AirlockPlus #7220

Merged
merged 1 commit into from
Jun 2, 2019

Conversation

cake-pie
Copy link
Contributor

@cake-pie cake-pie commented Jun 1, 2019

Since #7131 and #7200 are clearly going nowhere fast due to continued inertia, this is intended to provide some kind of workable interim arrangement so as to stop holding up my releases.

For now I will be bundling Harmony for manual installs in a manner similar to what I laid out in #7131 (comment) which follows the generally established convention for bundling Module Manager.

In this compromise approach, CKAN can take that bundled dll and put it in the mod folder instead of GameData to prevent collision between multiple mods that bundle Harmony in this way. This is effectively the same as the "suggestion" in #7131 that all mods that require Harmony to include their "own" copy of it.

For reasons already discussed in #7131 this is not my preferred approach; I cannot guarantee that this won't run into problems down the line. #7200 is still the more robust and recommended solution.

Syntax check requested before merge.
For sample refer to prerelease version.

@DasSkelett
Copy link
Member

DasSkelett commented Jun 1, 2019

@HebaruSan why does the rebuild not work here?
It still throws the Kopernicus file name error.

Edit: it needs to be rebased, doesn't it?

@HebaruSan
Copy link
Member

it needs to be rebased, doesn't it?

Yes.

@DasSkelett
Copy link
Member

Thanks.
@cake-pie, can you rebase this branch on the current master?
The tests are failing due to an unrelated problem that got fixed by now.

@DasSkelett
Copy link
Member

Thank you. Running netkan locally works fine, the Harmony dll is installed correctly:
installed files

The validation fails because it doesn't check the pre-release, and the last full release had no Harmony. But the regex looks good to me.

@HebaruSan any objections? Else I would merge this now.

@HebaruSan
Copy link
Member

@DasSkelett no objections. Also I'm not in charge of anything, you don't need my approval. :)

@DasSkelett
Copy link
Member

@DasSkelett no objections. Also I'm not in charge of anything, you don't need my approval. :)

Okay, just wanna ask if you spotted something that won't work, better know it before than fixing it afterwards :)

@DasSkelett DasSkelett changed the title Compromise: use Harmony bundled with AirlockPlus Use Harmony bundled with AirlockPlus Jun 2, 2019
@DasSkelett DasSkelett merged commit 5cd07bd into KSP-CKAN:master Jun 2, 2019
@DasSkelett
Copy link
Member

@cake-pie this is merged, feel free to finally release the new version.

SirMortimer added a commit to Kerbalism/Kerbalism that referenced this pull request Jun 4, 2019
This follows the preliminary (?) convention that seems to be the consensus
in KSP-CKAN/NetKAN#7220
@cake-pie cake-pie deleted the airlockplus11 branch June 8, 2019 17:57
gotmachine pushed a commit to gotmachine/Kerbalism-1 that referenced this pull request Jun 17, 2019
This follows the preliminary (?) convention that seems to be the consensus
in KSP-CKAN/NetKAN#7220
@sarbian sarbian mentioned this pull request Dec 28, 2020
2 tasks
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.

3 participants