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

Cleanup project #2182

Merged
merged 11 commits into from
Nov 15, 2017
Merged

Cleanup project #2182

merged 11 commits into from
Nov 15, 2017

Conversation

Olympic1
Copy link
Member

@Olympic1 Olympic1 commented Nov 10, 2017

Cleaned up the repo by removing unused references. Also fixed the paths in the powershell build script.

I need some feedback on the powershell build script.

Changes:

  • Removed remaining .gitattributes and .gitignore files, so we use the global ones
  • Removed a pattern in .gitattributes
  • Formatted App.config's
  • Updated ToolsVersion to 15.0
  • Updated to MSBuildToolsPath, as MSBuildBinPath was deprecated
  • Sorted Include's alphabetically
  • AutoUpdate
    • Renamed LICENSE to LICENSE.md
    • Removed log4net
    • Removed Newtonsoft.Json
    • Removed Microsoft.CSharp
  • CmdLine
    • Updated CommandLineParser to 1.9.71.2 (now uses .NET 4.5)
  • Core
    • Updated TxFileManager to use .NET 4.5
    • Updated SharpZipLib to 0.86.5.1
    • Removed CommandLine
    • Removed ChinhDo.Transactions.FileManager
    • Removed some System.* references
  • GUI
    • Added target back in that were previously removed
    • Removed Microsoft.CSharp
    • Removed some System.* references
    • Removed duplicate references
    • Removed userInstalled tag, as this was deprecated 2 years ago
  • NetKAN
    • Updated SharpZipLib to 0.86.5.1
  • Tests
    • Updated SharpZipLib to 0.86.5.1
    • Updated Moq to 4.7.145
    • Updated NUnit to 3.9.0
    • Removed System.Configuration
    • Removed references that don't exist anymore
    • Added some missing references
  • Fixed included .version files

@Olympic1 Olympic1 added Architecture Build Issues affecting the build system Pull request labels Nov 10, 2017
@Olympic1 Olympic1 self-assigned this Nov 10, 2017
Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

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

I don't have a Windows build environment, so I can't test the ps1, but the rest of it looks fine.

Does the new csproj in #2177 need any updates along these lines?

@Olympic1
Copy link
Member Author

I'll check and make a PR on you repo is it needs some changes

Update to MSBuildToolsPath
Update builds
Format App.config
Fix included version files
Cleanup last bits in project files
@Olympic1
Copy link
Member Author

Cleaned up last bits and updated some nuget packages.
Ready for merge.

@HebaruSan
Copy link
Member

Do you want another reviewer to take a look? I'd be fine with merging this, but I don't want to short-circuit the review process if that's not how things are supposed to be done.

@Olympic1
Copy link
Member Author

I definitely want someone to review the powershell build script before merging this.

Olympic1 added a commit to Olympic1/CKAN that referenced this pull request Nov 13, 2017
@Olympic1 Olympic1 mentioned this pull request Nov 13, 2017
build.ps1 Outdated
$CakeVersion = (Select-Xml -Xml ([xml](Get-Content $PackagesConfigFile)) -XPath "//package[@id='Cake'][1]/@version").Node.Value
$CakeExe = "${PackagesDir}/Cake.${CakeVersion}/Cake.exe"
$CakeExe = "${PackagesDir}\Cake.${CakeVersion}\Cake.exe"

Copy link
Member

Choose a reason for hiding this comment

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

Since Windows is fine with the forward slashes, but Linux treats them as escape characters, it makes sense to me to use forward slashes to make the two build scripts easy to keep in sync. Is there a particular reason to switch to backslashes other than "that's how Windows users see paths?"

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this change isn't really necessary as both forward- and backslashes are valid path separators on Windows.

HebaruSan pushed a commit to HebaruSan/CKAN that referenced this pull request Nov 14, 2017
@Olympic1
Copy link
Member Author

Reverted the slashes in the build script.

Unless someone wants some more changes, this is ready to merge.

@politas politas merged commit ff255ec into KSP-CKAN:master Nov 15, 2017
politas added a commit that referenced this pull request Nov 15, 2017
@Olympic1 Olympic1 deleted the Cleanup branch November 15, 2017 16:06
HebaruSan pushed a commit to HebaruSan/CKAN that referenced this pull request Nov 18, 2017
HebaruSan pushed a commit to HebaruSan/CKAN that referenced this pull request Nov 24, 2017
HebaruSan pushed a commit to HebaruSan/CKAN that referenced this pull request Nov 27, 2017
HebaruSan pushed a commit to HebaruSan/CKAN that referenced this pull request Nov 29, 2017
HebaruSan pushed a commit to HebaruSan/CKAN that referenced this pull request Dec 1, 2017
HebaruSan pushed a commit to HebaruSan/CKAN that referenced this pull request Dec 2, 2017
HebaruSan pushed a commit to HebaruSan/CKAN that referenced this pull request Dec 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture Build Issues affecting the build system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants