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

Declares a conflict with TweakScaleRescaled-Redist #9977

Merged

Conversation

Lisias
Copy link
Contributor

@Lisias Lisias commented Apr 6, 2024

TweakScaleRescaled-Redist is being offered as an option when installing TweakScale, whats a mistake - while I can guarantee compatibility with the current interfaces, I can't say the same for future new ones.

@HebaruSan
Copy link
Member

@JonnyOThan, that's your mod. Does this look right to you?

@JonnyOThan
Copy link
Contributor

JonnyOThan commented Apr 7, 2024

@Lisias What are you trying to do here? TweakscaleRescaled-Redist already has a conflict with Tweakscale-redist so not sure why this is needed. Did you mean to declare a conflict between Tweakscale and tweakscaleRescaled-redist?

er, I’d assume “yes” because that’s what the PR description says, but that’s not what this change does.

I have no problem merging this PR but it doesn’t actually do anything so I’m trying to figure out what you intended.

@KSP-CKAN KSP-CKAN deleted a comment from Lisias Apr 7, 2024
@Lisias
Copy link
Contributor Author

Lisias commented Apr 7, 2024

When I installed TweakScale™ via CKAN, CKAN asked me if I wanted to install TweakScale-Redist or TweakScaleRescaled-Redist , what's an error.

If TweakScale is being installed, the only option available should be TweakScale-Redist, and it should not be installed without it, or using the alternative.

@JonnyOThan
Copy link
Contributor

When I installed TweakScale™ via CKAN, CKAN asked me if I wanted to install TweakScale-Redist or TweakScaleRescaled-Redist , what's an error.

If TweakScale is being installed, the only option available should be TweakScale-Redist, and it should not be installed without it, or using the alternative.

That makes sense, but this PR doesn't do that. You would need to declare that TweakScale conflicts with TweakScaleRescaled-Redist.

@JonnyOThan
Copy link
Contributor

JonnyOThan commented Apr 7, 2024

By the way, having two versions of Tweakscale-redist does create a bit of a headache. What would you think about collaborating on that library so that there's only one version? Have you looked over the changes I made in tweakscalerescaled-redist?

@Lisias
Copy link
Contributor Author

Lisias commented Apr 7, 2024

TweakScale 2.5 will introduce a new interface, with retro-compatibility. Merging the Redists will bring over us yet more headaches than we have nowadays.

I'm fixing the pull request.

@JonnyOThan JonnyOThan merged commit 2c000fb into KSP-CKAN:master Apr 7, 2024
1 check passed
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.

None yet

3 participants