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

Safer build updates #3114

Merged
merged 1 commit into from
Jul 14, 2020
Merged

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Jul 14, 2020

This is #3108, minus:

  • The AWSSDK upgrade, because it broke the Inflator (see [Bug] netkan.exe no longer able to successfully use IAM  #3112)
  • The switch from packages.config to PackageReferences, because it created build files outside the _build folder and broke builds when switching from one branch to another
  • The SharpZipLib update, because it gave me some errors about 'ZipFile': type used in a using statement must be implicitly convertible to 'System.IDisposable'. 🖕
  • The CommandLineParser upgrade, because it created an inconsistency in which version of the library we're using (Netkan was upgraded, CmdLine wasn't; let's hold off till everything is ready)

Now some things are upgraded but everything should still work. The original motivation of being able to open Core in Visual Studio is probably satisfied, as most of those changes are preserved.

@techman83 can you please try building this and see if it works in the Inflator?

@HebaruSan HebaruSan added Pull request Build Issues affecting the build system Netkan Issues affecting the netkan data labels Jul 14, 2020
@HebaruSan HebaruSan requested a review from techman83 July 14, 2020 04:46
@HebaruSan HebaruSan force-pushed the feature/build-updates branch from 60ba994 to 04132bb Compare July 14, 2020 04:49
@techman83
Copy link
Member

Looks to be working!

447 [1] DEBUG CKAN.NetKAN.Processors.QueueHandler (null) - Initializing SQS queue handler
489 [1] DEBUG CKAN.NetKAN.Processors.Inflator (null) - Initializing inflator
707 [1] INFO CKAN.KSPManager (null) - Loading KSP instances from registry
749 [1] INFO CKAN.NetKAN.Processors.Inflator (null) - Using user-supplied cache at /home/netkan/ckan_cache
1169 [1] DEBUG CKAN.NetKAN.Processors.QueueHandler (null) - Looking up URL for queue Inbound.fifo
1319 [1] DEBUG CKAN.NetKAN.Processors.QueueHandler (null) - Looking up URL for queue Outbound.fifo
1329 [1] DEBUG CKAN.NetKAN.Processors.QueueHandler (null) - Queue URLs: <>
1330 [1] DEBUG CKAN.NetKAN.Processors.QueueHandler (null) - Looking for messages from <>

Copy link
Member

@techman83 techman83 left a comment

Choose a reason for hiding this comment

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

From a "doesn't break the inflator" perspective, looks ok to me. But I'm pretty unfamiliar with the Cake/C# build process.

@HebaruSan HebaruSan merged commit 799d9c4 into KSP-CKAN:master Jul 14, 2020
@HebaruSan HebaruSan deleted the feature/build-updates branch July 14, 2020 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Issues affecting the build system Netkan Issues affecting the netkan data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants