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

use simpler 'release' instead of 'non-prerelease' word in user-facing… #239

Merged
merged 3 commits into from Dec 9, 2022

Conversation

pastey
Copy link
Contributor

@pastey pastey commented Nov 9, 2022

… output

Hi,

Not sure if you'll find this PR useful and corresponding to your philosophy, but anyway...

I find current phrasing Latest non-prerelease version available is... a bit too complex. I would suggest to simplify it by replacing non-prerelease to plain... release.

I see that the same NonPrerelease term is used in variable names. If you find this suggestion feasible, then I can rename variables too.

Maybe you'll tell me that there's some deep idea behind non-prerelease, then I'm fine with that too :)

Thanks for the great tool that saves ton of my time!

@pastey pastey requested a review from a team as a code owner November 9, 2022 13:37
Copy link
Contributor

@MattKiazyk MattKiazyk left a comment

Choose a reason for hiding this comment

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

I aggee @pastey - can we change the enum as well so devs working on the project are less confused too!

@@ -62,7 +62,7 @@ public final class XcodeInstaller {
case let .unavailableVersion(version):
return "Could not find version \(version.appleDescription)."
case .noNonPrereleaseVersionAvailable:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't agree with you more! Can we go the next step and rename this enum as well?

@@ -147,7 +147,7 @@ You'll need Xcode 13 in order to build and run xcodes.

<details>
<summary>Using Xcode</summary>
Even though xcodes is a command-line app, lll of the normal functionality works in Xcode, like building, running, and running tests. You can even type text into Xcode's console when it prompts you for input like your Apple ID or 2FA code.
Even though xcodes is a command-line app, all of the normal functionality works in Xcode, like building, running, and running tests. You can even type text into Xcode's console when it prompts you for input like your Apple ID or 2FA code.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed a small typo here. Not related to the PR, but would be nice to fix 🙏

@pastey
Copy link
Contributor Author

pastey commented Nov 15, 2022

@MattKiazyk, please check 🙏

@MattKiazyk
Copy link
Contributor

Sorry @pastey can you rebase with the recent changes with runtime installs. Looks good otherwise.

@pastey
Copy link
Contributor Author

pastey commented Nov 18, 2022

Sorry @pastey can you rebase with the recent changes with runtime installs. Looks good otherwise.

Done. Hope I did it correctly (I'm more used to plain old merge)

@pastey
Copy link
Contributor Author

pastey commented Nov 24, 2022

Hey, @MattKiazyk will you have a chance to take one more look at this PR?

1 similar comment
@pastey
Copy link
Contributor Author

pastey commented Dec 9, 2022

Hey, @MattKiazyk will you have a chance to take one more look at this PR?

@MattKiazyk
Copy link
Contributor

Thanks for the change!

@MattKiazyk MattKiazyk merged commit a2039d6 into XcodesOrg:main Dec 9, 2022
@MattKiazyk MattKiazyk added the enhancement New feature or request label Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants