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

userguide: http.cookie keyword update v1 #8366

Closed

Conversation

jmtaylor90
Copy link
Contributor

Make sure these boxes are signed before submitting your Pull Request -- thank you.

Link to redmine ticket:

Describe changes:

#suricata-verify-pr:
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch:

Copy link
Contributor

@jufajardini jufajardini left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks! We have a redmine ticket for updating HTTP keywords in the userguide, so I have assigned it to you: https://redmine.openinfosecfoundation.org/issues/3025

We're trying to enforce the <80 character limit to lines for new contributions, but I wouldn't want to prevent this from being approved because of that, so just pointing out for next contributions.

Imo, there is no need to split this type of contribution into two PR, as both are connected to updating the same keyword's documentation. Again, adding this note for future contributions. :)

@jmtaylor90
Copy link
Contributor Author

Looks good to me, thanks! We have a redmine ticket for updating HTTP keywords in the userguide, so I have assigned it to you: https://redmine.openinfosecfoundation.org/issues/3025

We're trying to enforce the <80 character limit to lines for new contributions, but I wouldn't want to prevent this from being approved because of that, so just pointing out for next contributions.

ack

Imo, there is no need to split this type of contribution into two PR, as both are connected to updating the same keyword's documentation. Again, adding this note for future contributions. :)

It wouldn't be a big deal for me, I can fix up the character limit and squash the commits and resubmit if you would like?

@jufajardini
Copy link
Contributor

Looks good to me, thanks! We have a redmine ticket for updating HTTP keywords in the userguide, so I have assigned it to you: https://redmine.openinfosecfoundation.org/issues/3025
We're trying to enforce the <80 character limit to lines for new contributions, but I wouldn't want to prevent this from being approved because of that, so just pointing out for next contributions.

ack

Imo, there is no need to split this type of contribution into two PR, as both are connected to updating the same keyword's documentation. Again, adding this note for future contributions. :)

It wouldn't be a big deal for me, I can fix up the character limit and squash the commits and resubmit if you would like?

If that's ok with you, then I appreciate it. A few less lines to be fixed in the future :P

@jmtaylor90
Copy link
Contributor Author

Continued in #8370

@jmtaylor90 jmtaylor90 closed this Jan 12, 2023
@jmtaylor90 jmtaylor90 deleted the userguide-http-cookie-update-v1 branch February 6, 2023 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants