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

[Plugin Generator] Update template files #9193

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ycombinator
Copy link
Contributor

I recently (about a couple weeks ago) created a new logstash filter plugin using the plugin generator (bin/logstash-plugin generate ...). In the PR review for my plugin I got some feedback on elements that were generated by the plugin generator. So I figured I should address that feedback in the plugin generator templates as well, in order to aid future plugin authors who use the plugin generator. This PR does that.

Specifically, this PR:

  • Updates the instructions about how we build docs to no longer say that we parse comments from plugin code.
  • Adds a snippet to the Gemfile that allows testing against multiple branch versions.
  • Adds the config file for Travis CI.

Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

Doc changes LGTM. However, I'm wondering if the plugin generator also needs to be modified to create the index.asciidoc skeleton files for each plugin type. I've created PRs to update the example repos, so you could take the files from there. TBH, I haven't used the generator for a long time, so I'm not even sure what it spits out (or should spit out) these days. Here are the PRs:

logstash-plugins/logstash-input-example#12
logstash-plugins/logstash-codec-example#9
logstash-plugins/logstash-output-example#7
logstash-plugins/logstash-filter-example#9

@ycombinator
Copy link
Contributor Author

Sure, happy to throw skeleton index.asciidoc files for each plugin as part of this PR. I assume I can just copy over the ones from the links you pasted?

- rvm: jruby-9.1.13.0
env: LOGSTASH_BRANCH=6.x
- rvm: jruby-9.1.13.0
env: LOGSTASH_BRANCH=6.0
Copy link
Contributor

Choose a reason for hiding this comment

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

For the template, I would suggest to leave off 6.0. I need to update (add or replace) to 6.2 across the board. for the template, I think master, 6.x, and 5.6 have a decent longevity.

@jakelandis
Copy link
Contributor

One minor (optional) suggestion around the travis template, and +1 for the skeleton index.asciidoc.

LGTM , and thanks for taking care of this !

@andrewvc
Copy link
Contributor

Jenkins, please test this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants