Add optional notes to firewall rules #65
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nstapelbroek
commented
Mar 19, 2019
| @@ -198,7 +200,7 @@ func (c *Client) GetFirewallRules(groupID string) ([]FirewallRule, error) { | |||
| // protocol must be one of: "icmp", "tcp", "udp", "gre" | |||
| // port can be a port number or colon separated port range (TCP/UDP only) | |||
| func (c *Client) CreateFirewallRule(groupID, protocol, port string, | |||
| network *net.IPNet) (int, error) { | |||
| network *net.IPNet, notes string) (int, error) { | |||
There was a problem hiding this comment.
After reading #40 I think we should maybe create a CreateFirewallRuleWithNote and leave this method signature untouched. What do you think?
There was a problem hiding this comment.
No, I think it's fine this way. We'll just bump the major version number since its a breaking aPI change, but not really a big deal. Anyone using external golang code as a library should vendor his dependencies imho, and thus not have any problem with such changes. 😄
q8webmaster
approved these changes
Mar 19, 2019
|
@nstapelbroek Thanks a lot, looks good. |
|
Actually merged it already. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Heya 👋
Thank you for creating and sharing this repository!
Recently, Vultr added the option to leave a small note on a firewall rule. Extra convenient when you are trying to remember who a certain entry belongs to.
This PR should allow users to use this new notes functionality by adding an optional
--notesflag to the firewall rule creation command. Let me know what you think.The functional result:
Notes / questions:
I'm still a bit new to the language, so if you have any comments or suggestions: let me know! I've written down some thoughts below. Maybe you can help me out with these?
CreateFirewallRulemore gracefully? I don't think that optional arguments is a thing in golang 🤔