Skip to content

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!

return "Missing password. Please try again."
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?

<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 pastey requested a review from MattKiazyk November 15, 2022 09:19
@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.

2 participants