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

refactor: add link to helper docs when alternate contains patterns #1674

Conversation

snosratiershad
Copy link
Contributor

As discussed in this issue and this PR, I added # Alternate optional contains patterns, see <link to helper script docs> for more details in checkers where we have commented out strings in CONTAINS_PATTERNS with this link to helper script docs.

@peb-peb
Copy link
Contributor

peb-peb commented May 24, 2022

Looks good to me! (tagging @terriko for workflow approval)

You could remove the < > around the link, but that's just a personal preference. Let's see what others have to say on this.

@snosratiershad
Copy link
Contributor Author

I don't know what is the true practice about < >. I saw that in many of opensource projects when reference to license notice url in head of codebase files.
If it's not a good, please mention me to remove all of this signs.

@terriko
Copy link
Contributor

terriko commented May 24, 2022

I'm approving the github actions workflow to run tests. We haven't used the < and > characters around urls elsewhere so I'd probably leave them out for consistency, but I admit I don't feel very strongly about it.

@snosratiershad snosratiershad changed the title add # Alternate optional contains patterns, see <link to helper script docs> for more details in checkers where we have commented out strings in CONTAINS_PATTERNS add # Alternate optional contains patterns, see <link to helper script docs> for more details May 25, 2022
@snosratiershad snosratiershad changed the title add # Alternate optional contains patterns, see <link to helper script docs> for more details patterns: add link to helper docs when alternate contains patterns May 25, 2022
@snosratiershad
Copy link
Contributor Author

renaming PR for gitlint

@snosratiershad snosratiershad force-pushed the add-link-to-helper-script-doc-on-contains-patterns branch from 43f93e6 to 464d0ee Compare May 25, 2022 06:00
@snosratiershad
Copy link
Contributor Author

all "<", ">" removed

@b31ngd3v
Copy link
Contributor

hi @snosratiershad i don't think patterns is a valid conventional commit type, can you replace it with refactor?
and if you are curious and want to learn more, visit https://www.conventionalcommits.org/en/v1.0.0/.

@snosratiershad
Copy link
Contributor Author

Hey @b31ngd3v, thanks for the link, it was very helpful and I didn't know anything about it. I will fix it.

@snosratiershad snosratiershad changed the title patterns: add link to helper docs when alternate contains patterns refactor: add link to helper docs when alternate contains patterns May 25, 2022
@snosratiershad
Copy link
Contributor Author

@terriko, I discovered pipelines and don't think checks failure are related to changes in this PR, if I'm wrong please let me know and I will fix them.

@terriko
Copy link
Contributor

terriko commented May 26, 2022

Updating the branch to re-run tests. I'll try to get back to look at this after my meeting.

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

And the tests are behaving again! (I think you got hit with the conflict in the exploited cves patch and the reportlab test patch). Thank you for doing this, I'll go ahead and get these merged now.

@terriko terriko merged commit 286d9c9 into intel:main May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants