search

cegredev / josi

A Java library designed to make making decisions based on the current operating system easier.
MIT License
36 stars 4 forks source link

Add isAtLeast(OS) method #5

Closed dbwiddis closed 3 years ago

dbwiddis commented 3 years ago

Fixes #3

Makes the assumption the "Unknown" versions are newer, which might not be true but is more likely to be true than not.

Used your same formatting with tabs, which are evil. ;)

dbwiddis commented 3 years ago

So two methods; the isAtLeast() just returning false on family mismatch (or unknown family) or incorrect version alignment, and enforceAtLeast() doing similar but with exceptions (and probably just calling isAtLeast() internally)?

cegredev commented 3 years ago

So two methods; the isAtLeast() just returning false on family mismatch (or unknown family) or incorrect version alignment, and enforceAtLeast() doing similar but with exceptions (and probably just calling isAtLeast() internally)?

Yes, exactly! :)

cegredev commented 3 years ago

About this: I'm not sure if you missed my last comment...

Yes, but that is only the case if you just consider the family of the operating system. Take Solaris for example. As of right now, it is the only member of the OTHER family (besides UNKNOWN of course). If I were to say OS.SOLARIS.isAtLeast(OS.SOLARIS) that would return false, even though that's clearly wrong.

I agree that OS.UNKNOWN and OS.UNKNOWN should return false, but not OS.Family.OTHER and OS.Family.OTHER.

...but that's the reason I haven't merged this yet. I'd have no problem merging and then implementing this little fix myself, but before I do that I wanted to make sure I don't go over your head, in case you do actually want to do this yourself or disagree.

dbwiddis commented 3 years ago

I think I got confused by UNKNOWN (OS) vs. OTHER (Family). Really the correct solution is to put SOLARIS in a UNIX family along with AIX (which currently matches into LINUX). Although you can change that conditional to just an UNKNOWN OS. I'm fine if you want to tweak the logic after you merge (or even push to the PR fork before you merge... if you haven't learned how to do that as a maintainer, you should! )

cegredev commented 3 years ago

put SOLARIS in a UNIX family along with AIX

Yep, I'll implement that soon.

push to the PR fork before you merge... if you haven't learned how to do that as a maintainer, you should!

Just did that. I really am trying my best to do a good job of this, even though I'm new to all of it, believe me. I look forward to learning a lot more as this project progresses, haha.

dbwiddis commented 3 years ago

I look forward to learning a lot more as this project progresses, haha.

It's really one of the best places out there to self-learn software engineering! Try getting hired as a senior software engineer based primarily on 7 years of maintaining an open source project. ;)

cegredev commented 3 years ago

Haha, sounds like a good plan, see you in 7 years then ;)