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

Make system property and file operations injectable #47

Merged

Conversation

voidzcy
Copy link
Contributor

@voidzcy voidzcy commented Jan 19, 2021

Creates interfaces for abstracting away system property and file
operations. Wrapper plugins can inject their own code for
performing those operations in ways compliant to the
environment/tool.

See details in #48.

/cc @ejona86

src/main/java/kr/motd/maven/os/Detector.java Outdated Show resolved Hide resolved
@@ -245,33 +257,33 @@ private static String normalize(String value) {
return value.toLowerCase(Locale.US).replaceAll("[^a-z0-9]+", "");
}

private static LinuxRelease getLinuxRelease() {
private static LinuxRelease getLinuxRelease(FileActionFacade fileActionFacade) {
Copy link

Choose a reason for hiding this comment

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

Since this and similar are private, it may make sense to make them non-static instead of passing in the providers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. I was unsure about determineBitness() as it was public (I turned it to private and thought it would be easy to flip it back if they were intended to be public APIs).

Anyway, now I changed those static utilities to no non-static helpers. @trustin Please let me know if the change to determineBitness() is broken.

Copy link
Owner

@trustin trustin left a comment

Choose a reason for hiding this comment

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

Looking nice! Left one comment. Thanks, @voidzcy 🙇

src/main/java/kr/motd/maven/os/FileOperationProvider.java Outdated Show resolved Hide resolved
@trustin trustin added this to the 1.7.0 milestone Jan 20, 2021
Copy link
Owner

@trustin trustin left a comment

Choose a reason for hiding this comment

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

Thanks, @voidzcy 🙇 When do you need a new release?

@voidzcy
Copy link
Contributor Author

voidzcy commented Jan 21, 2021

Thanks, @voidzcy 🙇 When do you need a new release?

We would need this for making changes to our Gradle plugins. But it isn't something super critical, so you can keep with your release schedule.

@voidzcy
Copy link
Contributor Author

voidzcy commented Feb 3, 2021

@trustin Hmm... If possible, could you make a release including this recently? 😃

@trustin trustin merged commit a23ab32 into trustin:master Feb 6, 2021
@trustin
Copy link
Owner

trustin commented Feb 6, 2021

@voidzcy Just released 1.7.0. 🎉

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.

3 participants