Skip to content

Refactor FileRepository#8

Merged
niezbop merged 3 commits intoDragonBox:masterfrom
niezbop:refactor/extract_unitypackage_decompression
Nov 8, 2017
Merged

Refactor FileRepository#8
niezbop merged 3 commits intoDragonBox:masterfrom
niezbop:refactor/extract_unitypackage_decompression

Conversation

@niezbop
Copy link
Copy Markdown
Member

@niezbop niezbop commented Nov 8, 2017

  • Extract .unitypackage extraction from FileRepository to a new class UnityPackageOpener.

  • Various cleaning of calls inside FileRepository

- Extract .unitypackage extraction from FileRepository to a new class
  UnityPackageOpener.

- Various cleaning of calls inside FileRepositor# Please enter the commit message for your changes. Lines starting
{
public class UnityPackageOpener
{
public void OpenUnityPackage(string archivePath, bool deleteAfterOpening = false)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would ExtractUnityPackage be a better name?

deleteAfterOpening
);
}
public void OpenUnityPackage(string archivePath, string destinationPath, bool deleteAfterOpening = false)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should say what happens when destinationPath doesn't exist.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Documentation-wise?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes

deleteAfterOpening
);
}
public void OpenUnityPackage(string archivePath, string destinationPath, bool deleteAfterOpening = false)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is deleteAfterOpening needed? What's the use case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

None actually. Removing it

@niezbop niezbop force-pushed the refactor/extract_unitypackage_decompression branch from 1659fce to 4fc3123 Compare November 8, 2017 15:01

namespace Uplift.Common
{
public class UnityPackageOpener
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe the API should be

new UnityPackage(filePath).Extract(destination)

Copy link
Copy Markdown
Member

@lacostej lacostej left a comment

Choose a reason for hiding this comment

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

Great. Thanks for the cleanups!

@niezbop niezbop merged commit d4999b4 into DragonBox:master Nov 8, 2017
@niezbop niezbop deleted the refactor/extract_unitypackage_decompression branch November 8, 2017 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants