haproxy addons #62
haproxy addons #62
Conversation
| DOCUMENTATION = """ | ||
| --- | ||
| module: pfsense_haproxy_frontend | ||
| version_added: "2.10" |
There was a problem hiding this comment.
This should be 0.6.0 at the moment.
There was a problem hiding this comment.
yep i made this over a year ago and never did a pr. client got back to me an dnow i'm having to get it to work again. i'll fix that.
| #!/usr/bin/python | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
| # Copyright: (c) 2021 Chris Morton, cosmo@cosmo.2y.net |
There was a problem hiding this comment.
Might want to update the year.
|
thats neat earlier wheni pusehd /created the pr it didn't run the builds. now i can see the failures |
|
Thanks for the contribution. However in addition to fixing the checks, I would want to see some unit tests as well for merging this. |
Yeah, I have to approve the test runs manually. |
yeah i figured as much. i pushed it up anyway for others to use. the tests came from actually implementing it. i'll see if can find time to follow how you have done tests and add some. |
TODO: unit tests for haproxy frontend
|
I would encourage you to create a topic branch for PRs instead of using master. |
| @@ -140,3 +140,4 @@ def parse_interface(self, interface, fail=True, with_virtual=True): | |||
| if fail: | |||
| self.module.fail_json(msg='%s is not a valid interface' % (interface)) | |||
| return None | |||
|
|
|||
There was a problem hiding this comment.
Drop the blank line, though I suspect the sanity test will flag this as well.
|
|
||
|
|
There was a problem hiding this comment.
This seems like a mistake
|
Also as a heads up, I'm likely going to move the haproxy stuff to it's own collections. |
|
ok i haven' yhad a chance at all to look at what is required to get this merged. i will try this week. |
hmm. i usually just create PRS from my local branch/feature branch to develop or main. are you suggesting that i do a pr from my fork branch to the saem branch on your fork? (ie the topic branch?) i ahven't done that before and apologize for the immediate ignorance on this. i will research this as well. thanks |
You filed a PR from the master branch of your fork. You should create a new branch in your fork, say |
Yup, that's the plan. |
yeah. i had it on a feature branch, merged that into my master, then requsted the pr back to yours. so my bad. thanks. |
|
Okay, I've setup https://github.com/pfsensible/haproxy so please submit your PR there. Thanks. |
|
Moved to pfsensible/haproxy#1 |
these aren't clean. but if anyone wants to at least use them they work