Skip to content

Conversation

Cherrypick14
Copy link

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

Contribution style:

Our Contribution agreements:

Changes (if applicable):

Link to ticket: https://redmine.openinfosecfoundation.org/issues/6406

Describe changes:

  • Removed ambiguous "we" usages from userguide documentation across 5 files.
  • Updated quickstart.rst with imperative forms ("Install" instead of "We recommend").
  • Updated rules/intro.rst to use passive voice and system-focused language.
  • Updated code-style.rst to clarify project policy ("Suricata uses" instead of "We use").
  • Fixed grammar error in file-extraction.rst ("will be placed" instead of "we be placed").
  • Updated flow-keywords.rst to replace user-referring "we" with direct "you" or passive voice.

Provide values to any of the below to override the defaults.

  • To use a Suricata-Verify or Suricata-Update pull request,
    link to the pull request in the respective _BRANCH variable.
  • Leave unused overrides blank or remove.

SV_REPO=
SV_BRANCH=
SU_REPO=
SU_BRANCH=

@jufajardini jufajardini added the outreachy Contributions made by Outreachy applicants label Oct 16, 2025
@jufajardini jufajardini added the typo/doc update No code change : only doc or typo fixes label Oct 16, 2025
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.

Thanks for your contribution, there's progress here :)

Please check the inline comments, and also:

  • for the quickstart file, I'd say that likes 85 and 95 could also be adjusted
  • in the rules/intro.rst, section Direction could be worked upon, too
  • flow-keywords: did you review the other we usage cases? I see other sections with a similar situation

Since we know this touches many files, I would say that once we manage to complete the work started on these files, this can be seen as a first batch.

We will need a next iteration following our Code submission guidelines, especially with regards to commit message format and commit separation.

named `00` to `ff` where the directory shares the first 2 characters
of the filename. For example, if the SHA256 hex string of an extracted
file starts with "f9bc6d..." the file we be placed in the directory
file starts with "f9bc6d..." the file will be placed in the directory
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch! As this is as typo fix, could you please put this (and any other such fixes) in a separate commit, and clearly state that it is a commit fixing typos?

Copy link
Author

Choose a reason for hiding this comment

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

Hi @jufajardini sure, I'll do this. I think this is where I will have to leverage the cherrypick command right?

Copy link
Contributor

Choose a reason for hiding this comment

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

The cherry-pick command is more for when you want to bring a commit from a different branch to your branch.

In this case, two things can be done:

  • you can use git commit --amend to modify the last commit you've worked on
  • and you can use git commit --fixup to merge commits together


The dedicated PPA repository is added, and after updating the index, Suricata can
be installed. We recommend installing the ``jq`` tool at this time as it will help
be installed. Install the ``jq`` tool at this time as it will help
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one isn't needed. We as the team are indeed recommending that the user/ reader installs jq.

The purpose of the ticket is to remove ambiguous usages of we (where it isn't clear whether it refers to the team, or the user). This one isn't ambiguous, so it can stay. :)

Copy link
Author

Choose a reason for hiding this comment

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

Well noted.

fails per connection, but we have vulnerability where an attacker can
continue to login after that five attempts and we need to know about
Let's say you are tracking a protocol that normally allows five login
fails per connection, but has a vulnerability where an attacker can
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm thinking that maybe (this is a bit of polishing, but, still):

Suggested change
fails per connection, but has a vulnerability where an attacker can
fails per connection, but there is a vulnerability where an attacker can

continue to login after that five attempts and we need to know about
Let's say you are tracking a protocol that normally allows five login
fails per connection, but has a vulnerability where an attacker can
continue to login after that five attempts and you need to detect
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're here, how about we improve:

Suggested change
continue to login after that five attempts and you need to detect
continue to login after those five attempts and you need to detect

This could go in the typo-fixing commit ;)

Copy link
Author

Choose a reason for hiding this comment

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

I see another cherrypick usage here . Thanks for pointing this out.

are the options.

We will be using the above signature as an example throughout
The above signature serves as the above signature as an example throughout
Copy link
Contributor

Choose a reason for hiding this comment

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

This proposal doesn't look correct, and is not needed (similar explanation as in the other inline comment).

Copy link
Author

Choose a reason for hiding this comment

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

Got it. Will preserve the original wording.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

outreachy Contributions made by Outreachy applicants typo/doc update No code change : only doc or typo fixes

Development

Successfully merging this pull request may close these issues.

2 participants