Skip to content

Enable Bonsai Asset Build#62

Closed
terraboops wants to merge 3 commits intosensu-plugins:masterfrom
terraboops:feature/bonsai-asset
Closed

Enable Bonsai Asset Build#62
terraboops wants to merge 3 commits intosensu-plugins:masterfrom
terraboops:feature/bonsai-asset

Conversation

@terraboops
Copy link
Copy Markdown

@terraboops terraboops commented Jun 20, 2019

Pull Request Checklist

Adding Bonzai the same way it was done for this PR:
sensu-plugins/sensu-plugins-network-checks#94

General

  • Update Changelog following the conventions laid out here

  • Update README with any necessary configuration snippets

  • Binstubs are created if needed

  • RuboCop passes

  • Existing tests pass

Purpose

  • Enable Sensu Go Asset building
  • Remove unsupported ruby assets

Known Compatibility Issues

@terraboops
Copy link
Copy Markdown
Author

Sorry I don't know if Binstubs are needed...

Let me know if there's anything else you'd like me to change to get this merged. Thanks.

@terraboops
Copy link
Copy Markdown
Author

Ping!

I think this is a useful PR... I am using a fork of it from here and it's working well:
https://github.com/tylermauthe/sensu-plugins-consul/releases/tag/3.0.0

Anything else I can do to help get this merged @majormoses? 🙇

@majormoses majormoses requested a review from jspaleta August 29, 2019 23:08
Copy link
Copy Markdown
Member

@majormoses majormoses left a comment

Choose a reason for hiding this comment

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

LGTM

I would like a second pair of eyes on this one since I am less familiar with bonsai setup.

@nixwiz
Copy link
Copy Markdown

nixwiz commented Dec 27, 2019

@tylermauthe could you take a look at the .bonsai.yaml file in #64 and update this PR to include it? It has more build targets and it makes use of platform_family to provide better filtering.

@terraboops
Copy link
Copy Markdown
Author

@nixwiz - when I added that, the build broke
https://travis-ci.org/sensu-plugins/sensu-plugins-consul/builds/642666307

However, I have reverted my changes to the original and it seems the build is still broken. Is master building correctly? Has a new test been added?

Note: I haven't even looked at the error output - but I do notice that all recent builds on PRs are failing.

@majormoses
Copy link
Copy Markdown
Member

the failure is the version of bundler being installed in travis differs from what is specified in the gemspec:

We probably want to specify in the travis file to install/upgrade (a specific version) of bundler before attempting to bundle for example: https://github.com/sensu-plugins/sensu-plugins-windows/blob/83a67bf66a33c7b6762c3a7335284501aff1c1bb/.travis.yml#L5.

services:
- docker
cache:
- bundler
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add the following lines here to help fix Travis build

before_install:
- gem install bundler

Copy link
Copy Markdown

@nixwiz nixwiz left a comment

Choose a reason for hiding this comment

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

After updating .travis.yml, also update the bundler dependency in the gemspecl to '~> 2.1' and add these changes to the CHANGELOG.

@terraboops terraboops closed this Mar 6, 2021
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.

3 participants