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

Automate - Refactoring Enhanced Messaging methods. #20

Closed

Conversation

billfitzgerald0120
Copy link
Contributor

Changed all {updated_message += "?"} to {updated_message << "?"} in all 10 methods.

@billfitzgerald0120 billfitzgerald0120 changed the title Automate - Refactor Enhanced Messaging methods. Automate - Refactoring Enhanced Messaging methods. Jan 13, 2017
@billfitzgerald0120
Copy link
Contributor Author

@tinaafitz
Please Review

@tinaafitz
Copy link
Member

@billfitzgerald0120 Looks good.
@mkanoor Please review.

@billfitzgerald0120
Copy link
Contributor Author

@tinaafitz @mkanoor
this PR should wait till this refactored one gets merged.
#19

@miq-bot
Copy link
Member

miq-bot commented Jan 27, 2017

This pull request is not mergeable. Please rebase and repush.

@billfitzgerald0120
Copy link
Contributor Author

@tinaafitz
Please Review

@gmcculloug
Copy link
Member

@billfitzgerald0120 Lots of rubocop warnings. Why did you add the String.new line instead of keeping the previous logic of initializing the string with the first text message?

@miq-bot
Copy link
Member

miq-bot commented Feb 15, 2017

Checked commits billfitzgerald0120/manageiq-content@189fb1a~...5c29296 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
11 files checked, 0 offenses detected
Everything looks good. 🍰

@mkanoor
Copy link
Contributor

mkanoor commented Feb 15, 2017

Closed because these changes were done in anticipation of frozen strings in the next version of ruby.
We will make the changes when we get to the next version of ruby.

@mkanoor mkanoor closed this Feb 15, 2017
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.

5 participants