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.

Sign up for GitHub

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

Jump to bottom

Add install size to metadata and display in clients #3568

Merged
merged 2 commits into from
Aug 4, 2022
Merged

Add install size to metadata and display in clients #3568

merged 2 commits into from
Aug 4, 2022

Conversation

HebaruSan
Copy link
Member

@HebaruSan HebaruSan commented Apr 17, 2022 •
edited
Loading

Motivation

This looks a little silly, since each of these modules installs a different, smaller subset of the download:

image

And this isn't accurate because it sums the download_size properties of the installed modules, not their installed sizes:

image

Changes

  • Now an install_size property is added to the schema, spec, and CkanModule, and InstallSize is added to GUIModule
  • Now Netkan populates install_size based on the sum of the Size properties of the list of the ZipEntry objects that the module installs
  • Now the ConsoleUI uses this property instead of and falling back to download_size
  • Now a column for install size is added to GUI

@HebaruSan HebaruSan added Enhancement New features or functionality Easy This is easy to fix GUI Issues affecting the interactive GUI Core (ckan.dll) Issues affecting the core part of CKAN Pull request Spec Issues affecting the spec Netkan Issues affecting the netkan data Schema Issues affecting the schema ConsoleUI Issues affecting the interactive console UI labels Apr 17, 2022
@HebaruSan HebaruSan removed the Easy This is easy to fix label Apr 18, 2022
@HebaruSan

This comment was marked as outdated.

Sign in to view
techman83
techman83 approved these changes Aug 4, 2022
View reviewed changes
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.

Looks like a useful addition. I note that the bot will do a bunch of work once merged. Do we have any tests validating this transformer or will it fail gracefully?

@HebaruSan
Copy link
Member Author

Good point, I will have a look at writing some tests. Especially since it's a new transformer...

@HebaruSan HebaruSan merged commit 1118f5e into KSP-CKAN:master Aug 4, 2022
@HebaruSan HebaruSan deleted the feature/install-size branch August 4, 2022 03:05
@HebaruSan
Copy link
Member Author

Update of the original screenshot, now we can see how big each package is when decompressed and installed:

image

@HebaruSan HebaruSan mentioned this pull request Aug 4, 2022
Merged
@HebaruSan HebaruSan mentioned this pull request Aug 16, 2022
Merged
@HebaruSan HebaruSan mentioned this pull request Nov 17, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@techman83 techman83 techman83 approved these changes

@DasSkelett DasSkelett Awaiting requested review from DasSkelett

Assignees
No one assigned
Labels
ConsoleUI Issues affecting the interactive console UI Core (ckan.dll) Issues affecting the core part of CKAN Enhancement New features or functionality GUI Issues affecting the interactive GUI Netkan Issues affecting the netkan data Schema Issues affecting the schema Spec Issues affecting the spec
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@HebaruSan @techman83