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

Manual exceptions #120

Closed
xmulligan opened this issue Mar 3, 2022 · 17 comments
Closed

Manual exceptions #120

xmulligan opened this issue Mar 3, 2022 · 17 comments

Comments

@xmulligan
Copy link
Contributor

Is there a way to to manually override the checks? For instance at Cilium https://clomonitor.io/projects/cilium/cilium

We have a a USERS.md rather than ADOPTERS.md https://github.com/cilium/cilium/blob/master/USERS.md

and our governance file is in the documentation folder which wasn't part of the scan https://github.com/cilium/cilium/blob/master/Documentation/contributing/governance/commit_access.rst

@xmulligan
Copy link
Contributor Author

and we also have the changelog with each release notes https://github.com/cilium/cilium/releases/tag/v1.11.2

@tegioz
Copy link
Contributor

tegioz commented Mar 3, 2022

Hi @xmulligan 👋

There isn't any way to override locations for checks yet, but it's something we are considering. We recently introduced the .clomonitor.yml metadata file that we could extend for this purpose.

In addition to detecting the the ADOPTERS.md file, we also check if an Adopters header is present in the README.md. If adding a minimal section to your readme file pointing users to the USERS.md file sounds like a good option, it should do the trick 🙂 The Kubernetes project has handled it this way, for example. This works for some more checks, including Governance.

@tegioz
Copy link
Contributor

tegioz commented Mar 3, 2022

Regarding the changelog check, we are actually checking the notes of the last release looking for this pattern (?i)changelog. Maybe we could also check for an additional pattern with changes in a header title 🤔 The trick mentioned above would work for the changelog check as well though.

@caniszczyk
Copy link

for the ADOPTERS.md check, I think we should check for ADOPTERS.md or USERS.md @tegioz and just build that into the check

tegioz added a commit that referenced this issue Mar 3, 2022
Related to #120

Signed-off-by: Sergio Castaño Arteaga <[email protected]>
@tegioz
Copy link
Contributor

tegioz commented Mar 3, 2022

for the ADOPTERS.md check, I think we should check for ADOPTERS.md or USERS.md @tegioz and just build that into the check

Done 👍

tegioz added a commit that referenced this issue Mar 3, 2022
Related to #120

Signed-off-by: Sergio Castaño Arteaga <[email protected]>
@xmulligan
Copy link
Contributor Author

Thanks for the help! We are going to do something similar to Kubernetes see cilium/cilium#19037

@tegioz
Copy link
Contributor

tegioz commented Mar 4, 2022

I've just realized that your README file uses reStructured text, not markdown. At the moment we only support detecting those headers in markdown files @xmulligan.

@tegioz
Copy link
Contributor

tegioz commented Mar 4, 2022

The regular expressions we are using for headers look for the hash symbols used in markdown headers (IIRC headers in reStructured text are different). But I think maybe we could also accept those words (Governance, etc) when they appear at the beginning of a line and it's the only word on it. That should make it work for this case and it's generic enough to be ok.

I'm finishing something but as soon as I'm done I'll take a look at this, will keep you posted 👍

tegioz added a commit that referenced this issue Mar 4, 2022
Related to #120

Signed-off-by: Sergio Castaño Arteaga <[email protected]>
tegioz added a commit that referenced this issue Mar 4, 2022
Related to #120

Signed-off-by: Sergio Castaño Arteaga <[email protected]>
@tegioz
Copy link
Contributor

tegioz commented Mar 4, 2022

It's done @xmulligan, we should be able to detect those headers in README files using reStructured text 🙂

Will deploy it within the next hour 🚀

@xmulligan
Copy link
Contributor Author

Thanks @tegioz ❤️ really appreciate all the help 💯

@tegioz
Copy link
Contributor

tegioz commented Mar 4, 2022

No worries! 😊👋

@xmulligan
Copy link
Contributor Author

ok, sorry same question about the slack now 😅 We have been approved to keep the Cilium + eBPF slack separate from the CNCF or K8s slack. What can we do to meet the requirement?

@xmulligan xmulligan reopened this Mar 18, 2022
@tegioz
Copy link
Contributor

tegioz commented Mar 18, 2022

Hi @xmulligan

We have a few options to handle this:

@caniszczyk
Copy link

caniszczyk commented Mar 18, 2022 via email

@tegioz
Copy link
Contributor

tegioz commented Mar 18, 2022

Ok, cool 👍

Will close this issue in favor of #154 @xmulligan. You'll be able to add an exemption for this check as soon as #154 is ready.

@tegioz tegioz closed this as completed Mar 18, 2022
tegioz added a commit that referenced this issue Mar 22, 2022
Closes #154
Related to #120

Signed-off-by: Sergio Castaño Arteaga <[email protected]>
Signed-off-by: Cintia Sanchez Garcia <[email protected]>
Co-authored-by: Sergio Castaño Arteaga <[email protected]>
Co-authored-by: Cintia Sanchez Garcia <[email protected]>
tegioz added a commit that referenced this issue Mar 22, 2022
Closes #154
Related to #120

Signed-off-by: Sergio Castaño Arteaga <[email protected]>
Signed-off-by: Cintia Sanchez Garcia <[email protected]>
Co-authored-by: Sergio Castaño Arteaga <[email protected]>
Co-authored-by: Cintia Sanchez Garcia <[email protected]>
@tegioz
Copy link
Contributor

tegioz commented Mar 22, 2022

Support for exemptions is ready @xmulligan. Please see the checks document for more details.

@xmulligan
Copy link
Contributor Author

Thanks for the help, I've opened cilium/cilium#19235 to cover this 💯

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

No branches or pull requests

3 participants