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

Added support for Amazon Linux distribution in post remove scripts #1381

Merged
merged 1 commit into from
Jul 28, 2016

Conversation

chebrolus
Copy link
Contributor

Required for all PRs:

  • CHANGELOG.md updated
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

@sparrc
Copy link
Contributor

sparrc commented Jun 22, 2016

I'm not sure that you've handed all cases here. What if the script is called in "upgrade" mode on amazon linux? Why are you not detecting that? (see relevant if-statements for debian and rpm)

@chebrolus
Copy link
Contributor Author

Thanks for review and feedback @sparrc
Let me check Amazon equivalent for this check and put in so that only triggered in case of post package remove.

@chebrolus chebrolus force-pushed the master branch 2 times, most recently from f27da53 to 3321309 Compare June 24, 2016 01:16
@chebrolus
Copy link
Contributor Author

@sparrc - Please review the update and share feedback. Sorry I had to force push as I ran into issue with rebase from upstream.

Instead of going by the Linux distribution based conditional branching this revised script handle remove based on what packaging guidelines these distributions follow.
Looks like you have similar issue with InfluxDB too, if this is acceptable solution you can follow similar approach for InfluxDB as well.

@sparrc
Copy link
Contributor

sparrc commented Jul 19, 2016

thnaks @schebrolu

@sparrc
Copy link
Contributor

sparrc commented Jul 19, 2016

@schebrolu can you rebase your changes and re-push. Force pushing is totally fine, by the way.

@chebrolus
Copy link
Contributor Author

Thank you @sparrc
Rebased and pushed my changes, please review and merge.

@sparrc
Copy link
Contributor

sparrc commented Jul 25, 2016

thanks @schebrolu

@sparrc sparrc closed this Jul 25, 2016
@chebrolus
Copy link
Contributor Author

No merge :(

@sparrc sparrc reopened this Jul 28, 2016
@sparrc sparrc merged commit 841729c into influxdata:master Jul 28, 2016
@sparrc
Copy link
Contributor

sparrc commented Jul 28, 2016

oops! my mistake

aurrelhebert pushed a commit to aurrelhebert/telegraf that referenced this pull request Aug 9, 2016
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