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

feature: Rule methods #104

Open
greenpau opened this issue Aug 1, 2020 · 5 comments · May be fixed by #108
Open

feature: Rule methods #104

greenpau opened this issue Aug 1, 2020 · 5 comments · May be fixed by #108
Labels
enhancement New feature or request

Comments

@greenpau
Copy link
Contributor

greenpau commented Aug 1, 2020

@stapelberg, @sbezverk and other maintainers, currently many module's structs have no methods of their own. For example, it would have been nice to have String() method for Rule.

Is it OK to submit a PR for this?

@stapelberg
Copy link
Collaborator

Regarding the String() method, just to double-check: it would return a human-readable representation of the rule, and wouldn’t be used programmatically, yes? If so, that seems okay to me.

@stapelberg stapelberg added the enhancement New feature or request label Aug 2, 2020
@greenpau
Copy link
Contributor Author

greenpau commented Aug 2, 2020

Regarding the String() method, just to double-check: it would return a human-readable representation of the rule, and wouldn’t be used programmatically, yes?

@stapelberg , Yes. I want to be able to see one liner rule, similar to “nft” utility. Today, it takes an “essay” to get a gist of what a rule does! 😄 i am not complaining ... i appreciate the complexity ... i learned from the experience ... however, it would probably make the library easier to consume. In fact, back in March, i failed to use the library because of its use complexity. Then, I stumbled upon a few blog posts and had an epiphany :-)

@greenpau
Copy link
Contributor Author

greenpau commented Aug 2, 2020

It is just Rule does not have any method ... For example ... ‘IsVerdictDrop’, ‘IsVerdictJump’, ‘IsIPv4’, ‘IsIPv6’, etc.

@stapelberg
Copy link
Collaborator

Sure, in that case adding String() makes sense.

For the other methods, I’m tentatively in favor, but let’s discuss those over a pull request maybe?

In general, this library is pretty low-level, so I don’t want to add too much syntactic sugar.

@greenpau
Copy link
Contributor Author

greenpau commented Aug 3, 2020

For the other methods, I’m tentatively in favor, but let’s discuss those over a pull request maybe?

@stapelberg , absolutely!

In general, this library is pretty low-level, so I don’t want to add too much syntactic sugar.

👍

greenpau added a commit to greenpau/origin_google_nftables that referenced this issue Aug 3, 2020
Before this commit: the printing of a rule results in
a pointer address.

After this commit: the printing of a rules results in
a human-readable text.

Resolves: google#104

Signed-off-by: Paul Greenberg <greenpau@outlook.com>
@greenpau greenpau linked a pull request Aug 3, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants