search

nickwallen / metron-commit-stuff

2 stars 3 forks source link

Add support for apache/metron-bro-plugin-kafka #11

Closed JonZeolla closed 6 years ago

JonZeolla commented 6 years ago

This is to add support for apache/metron-bro-plugin-kafka to both the prepare-commit and checkout-pr scripts. It should default to apache/metron across the board, but allows you to specify metron-bro-plugin-kafka when prompted, which should propagate properly throughout the rest of the3 script. This also adds some checks for various scenarios I thought up during my updates.

JonZeolla commented 6 years ago

Are you suggesting something like:

Prompt_for_repo()
Check_cwd()
Clone_repo()
Check_folder_names()
Fetch_pr()

For checkout-pr, as an example?

ottobackwards commented 6 years ago

Yes, but suggest isn't the right word. I'm inclined. I don't think in bash. If it is proper form to NOT have functions for things like this, then no problem.

JonZeolla commented 6 years ago

I'm happy to refactor it if it would help its readability.

ottobackwards commented 6 years ago

let's see what @nickwallen thinks, it is, um his repo

nickwallen commented 6 years ago

@JonZeolla Did you want me to merge this PR here? Or would you prefer that I open the PR to move prepare-commit as-is over to Metron proper and then you can propose this PR there? It might get better testing if we do the latter.

JonZeolla commented 6 years ago

Either is fine with me. May be easier to merge first then PR against Metron. Just looking to mainstream this and get feedback

JonZeolla commented 6 years ago

Just realized I should update the README examples as a part of this PR. I'll try to get around to that this week sometime, once my other bro work is complete.

JonZeolla commented 6 years ago

Okay, given your comments @nickwallen I think I agree that this should be moved over first, then I'll submit my update.

While I was throwing together an updated README for my changes I noticed that the curl command(s) HTML encode the JIRA description, but I don't have a clean bash-native way to decode it. Wasn't sure if you had an easy fix for this or not - only mentioning as it may be an easy fix to throw in prior to the migration.