Add metric: security policy

[abdumamdouh]

PR for issue #73

I have added a metric prop for security in both metrics.json and metrics.js.

is that exactly what you meant? if not could you give me more information or hints

[ljharb]

@abdumamdouh https://docs.github.com/en/graphql/reference/objects#repository suggests that the security policy is indeed part of the graphql api as isSecurityPolicyEnabled.

[abdumamdouh]

Hello @ljharb security.md is contained in repo, for that I need exact link to repo to check if it is in repo, and repo report finds a list of all repos So, I need to check it for all of the repos ? If so, where it should be shown ??

[ljharb]

The file could be in either:

• $owner/$repo/SECURITY.md
• $owner/$repo/.github/SECURITY.md
• $owner/.github/SECURITY.md

• $owner/.github/.github/SECURITY.md

  If there’s not a way to search for these paths with GraphQL, then we’d have to make a curl request to https://github.com/$owner/$repo/security/policy for each repo, which isn’t going to be scalable, so we might not be able to proceed.

[abdumamdouh]

@ljharb this will work with the PR of "code of conduct" too, right? the difference is the file itself

[ljharb]

It would! Although it turns out the CoC info is in the graphql api after all.

[abdumamdouh]

hello @ljharb it works now

[abdumamdouh]

@ljharb i think it's ready to be merged

[ljharb]

The fix is great :-) but we still don't have any tests covering this fixture data, or else the PR would be failing them.

[abdumamdouh]

The fix is great :-) but we still don't have any tests covering this fixture data, or else the PR would be failing them.

@ljharb are you sure that there are no tests, the only failing test is "Automatic Rebase / Automatic Rebase (pull_request_target)"

I have checked both files metricConfig.json and fixtures.js inside the tests folder and there is SecurityPolicyEnabled is set to false in the metricConfig.json

am I missing something?

[ljharb]

@abdumamdouh yes, that's my point. this PR inverts a boolean, so i'd expect some tests to fail - but instead, they pass. That means we don't have good test coverage for this value.

[abdumamdouh]

@ljharb I think they pass because now it works correctly? I tried it

[ljharb]

But they didn't fail before - which proves they're not testing this.

[ljharb]

@abdumamdouh i'd love to land this if we could get a test added :-)

[abdumamdouh]

@ljharb tbh i don't know where to add a test for this, if you can give me a hint or something

[ljharb]

What I'd expect is two fixtures (real API responses):

1. a repo that has a security policy, like https://github.com/ljharb/tc39-ci
2. a repo that does not have a security policy, like https://github.com/tc39/ecma262

and then asserting that the first one shows up as "SecurityPolicyEnabled" being true, and the second false.

[abdumamdouh]

@ljharb can you review now?