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

Fix mysql::sql task error message #1243

Merged

Conversation

alexjfisher
Copy link
Collaborator

Because of misplaced " no interpolation of the error message was
actually taking place. After fixing this, the Lint/UselessAssignment
rubocop violation that had been disabled goes away too.

@alexjfisher alexjfisher requested a review from a team as a code owner October 14, 2019 17:19
alexjfisher referenced this pull request Oct 14, 2019
For whatever reason, rubocop cannot see that stderr is indeed a used variable.
@adreyer
Copy link

adreyer commented Oct 14, 2019

We also need to update the response to add a _error hash instead of or in addition to the error string so that error messages will be displayed correctly by task runners. Do you want to do that as part of this PR or should I put one up?

Because of misplaced `"` no interpolation of the error message was
actually taking place.

Rubocop had been warning about this bug, but was deactivated in both
tasks (in different ways and by two different people)
@alexjfisher
Copy link
Collaborator Author

@adreyer Would you like to? I'm not too confident with bolt at the moment. Similar fix needed in puppetlabs/postgresql (and probably other modules) too.

@alexjfisher
Copy link
Collaborator Author

@nicklewis I've found a similar bug in the other task export.rb. Do you want to request a review from @hunner too? ;)

@michaeltlombardi
Copy link
Contributor

Thanks for the fix @alexjfisher!

@michaeltlombardi michaeltlombardi merged commit ab9292e into puppetlabs:master Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants