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

refactor: clean up style and redundancy #3232

Merged
merged 2 commits into from
Jul 29, 2023
Merged

refactor: clean up style and redundancy #3232

merged 2 commits into from
Jul 29, 2023

Conversation

IgorEisberg
Copy link
Contributor

@IgorEisberg IgorEisberg commented Jul 29, 2023

There are still plenty of inconsistencies, clearly showing that different people wrote certain parts of code, but this should trim down quite some of them.

Highlights:

  1. The private instance methods in ApkBuilder were written like static methods by needlessly taking already-accessible instance fields as methods arguments.
  2. hasManifest, hasResources, hasSources and hasMultipleSources were moved to ApkInfo by having it accept an ExtFile instance that's saved to mApkFile in order to make these methods accessible across multiple classes without having to repeat them. Point to mention: mApkFile is not set if the ApkInfo was parsed from a stream (rather than a file), which is only used in tests using getResourceAsStream and not in actual source code, so it's not an issue. Another note: the mApkFile field is marked as transient just for compat purposes, it's not really necessary since you're now doing manual YAML serialization, so that field won't interfere either with or without that keyword,
  3. Some imports from common packages, like java.io and java.util were more verbose than necessary. Those are easily recognizable and don't really need special mentioning. Either we make them explicitly listed across the whole project, or implicitly used with a wildcard - I chose the second option, it's just cleaner.
  4. Some throws lists in certain method signatures appear on a new line despite the signature being rather short.
  5. Normalized some try-with-resources statements.

@iBotPeaches
Copy link
Owner

In future, if you don't use master as your submitting branch it makes my life easier. I cannot check this out easily with the gh pr checkout 3232 command when the incoming branch is master.

@IgorEisberg
Copy link
Contributor Author

In future, if you don't use master as your submitting branch it makes my life easier. I cannot check this out easily with the gh pr checkout 3232 command when the incoming branch is master.

Gotcha, didn't know about that.

@iBotPeaches iBotPeaches merged commit 33ca292 into iBotPeaches:master Jul 29, 2023
@iBotPeaches
Copy link
Owner

thanks! Ran a few tests and all worked fine. Will look into methods for enforcing some cs to help with imports, exception, newlines, etc

@iBotPeaches iBotPeaches added this to the v2.8.2 milestone Jul 30, 2023
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.

2 participants