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

Improved robustness of the update_gem function #31

Closed
wants to merge 1 commit into from

Conversation

gvatsal60
Copy link
Contributor

Improved robustness of the update_gem function.

  • Added error handling for gem command not found and update/cleanup failures.
  • Enhanced messages to provide clearer feedback to the user.
  • Introduced consistent return codes to signal errors in the script.

@gvatsal60
Copy link
Contributor Author

Hi @andmpel

Please review it!

Copy link
Owner

@andmpel andmpel left a comment

Choose a reason for hiding this comment

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

check gem command is unnecessary as gem is built into macOS, therefore it will always be there.

@gvatsal60
Copy link
Contributor Author

check gem command is unnecessary as gem is built into macOS, therefore it will always be there.

I agree, but it's possible that in the future, macOS might decide to remove gem. Therefore, to be better prepared and maintain consistency, it’s best to let it remain..

@andmpel
Copy link
Owner

andmpel commented Feb 10, 2025

I find it extremely unlikely that Apple would remove ruby/gem from their Unix based OS. I favor keeping the script smaller than adding redundant/unnecessary checks.

@gvatsal60
Copy link
Contributor Author

I find it extremely unlikely that Apple would remove ruby/gem from their Unix based OS. I favor keeping the script smaller than adding redundant/unnecessary checks.

After thorough consideration, I agree with the comment. Closing this pull request.

@gvatsal60 gvatsal60 closed this Feb 14, 2025
@gvatsal60 gvatsal60 deleted the fix/gem_update branch February 14, 2025 11:53
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