-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
docs: improve code documentation- v3 #9611
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @zazie-codes ! Thanks for incorporating the formatting changes, could you please squash all the commits together and make sure they are as per the guidelines: https://docs.suricata.io/en/latest/devguide/codebase/contributing/code-submission-process.html#commits
Check other folks's commits in the open approved PRs/merged in the repo to get examples. 😉
NOTE: This PR may contain new authors:
|
On it. Thank you. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #9611 +/- ##
==========================================
- Coverage 82.28% 82.26% -0.03%
==========================================
Files 968 968
Lines 274275 274275
==========================================
- Hits 225700 225638 -62
- Misses 48575 48637 +62
Flags with carried forward coverage won't be shown. Click here to find out more. |
Hi Shivani, instead of squashing the commits together i ended up adding another. Kindly advice, i followed the procedure but i must,ve skipped a process. |
Liza, this is a nice guide and on the lines of what was demonstrated by Jason yesterday in the live session. Please try this and see if you can figure out the steps you missed. It's important that you practice this as this will be needed for almost all open source projects. Good luck! :) |
Going through it |
07cfe9c
to
9c89dc4
Compare
Hi Shivani, I'm done. The squash was successful. |
Thanks, Liza! I've noticed that your git author is missing a surname, (we usually follow the format We also add the ticket number we're working on to the commit message, please see this commit as an example of what we expect: :) |
NOTE: This PR may contain new authors:
|
Yes I'm on it. |
ad3dd9d
to
305186f
Compare
Hi Juliana. I've made the changes. |
305186f
to
0a9c7b4
Compare
NOTE: This PR may contain new authors:
|
1 similar comment
NOTE: This PR may contain new authors:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed this PR looking at the previous reviews, and it seems that you have incorporated most of the feedback given. :)
I've also checked that commit style and author ID are conforming to our guidelines, now.
I've left three inline comments, but these should be straightforward to address, at this point :)
b904b4c
to
687020e
Compare
@zazie-codes you're supposed to make new PR each time to incorporate any changes 😉 |
Hi, I've incorporated the changes requested. |
Done. |
Thanks and kudos, but when incorporating feedback, especially when code-related, can you please submit those as part of a new PR, as you've done with the previous versions of your work? I know not all communities are like that, but this is really how our workflow goes, and it's easier for us to follow up and compare stuff. |
Sure Juliana...the changes are incorporated in the new PR #9650 |
Thanks for mentioning it, I'll close this one, then. |
Replaced by: #9649 |
NOTE: This PR may contain new authors:
|
Make sure these boxes are signed before submitting your Pull Request -- thank you.
Link to redmine ticket:https://redmine.openinfosecfoundation.org/issues/6383
Describe changes:
This is an update of #9606