OpenSSF Scorecards Biweekly, 17 Nov 2022

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Scorecards Biweekly Sync (November 17, 2022)

Description

https://github.com/ossf/scorecard#connect-with-the-scorecards-community

A

Hey how's, it going guys.

B

B

B

I think I'll just killed like a.

C

Few more minutes.

B

In case anyone wants to join.

B

D

There, hello, everyone hope everyone's doing well,.

B

Hi so uh yeah, let's um I'll I, guess I'll start it now and if anyone else's joining, we won't. We won't get too too far ahead, but yeah. uh So uh welcome uh so we're meeting. This is like the scorecards uh working group meeting.

B

um My name is raghav I I work uh at Google on on scorecards. um We don't have a super packed agenda today. So if there's like anything else that you feel that you'd want to add, please skip please go ahead, but uh yeah I'll be facilitating.

B

um If there's any one uh new who's like your first time coming to a scorecards uh meeting, uh yeah feel feel free to introduce yourself.

E

Teresa you're just first scorecard meeting welcome. Thank you. What.

D

Organization are you from.

E

um I work at sonotype and been following your projects for a while um until there was a meeting today, so I thought it might be a good time to like jump in um also to note that your the link to the document that is in the meeting minutes requires access.

D

You just joined the if you join the mailing list, it's automatic, but um no it's.

B

D

Okay, uh I might have the uh super power to add you hold up. Let me see, oh I. Think I do can uh do you mind, saying your email address out loud or is that yeah.

E

C

Should be tied to the group.

D

uh How is this, why don't you stick it in chat like you? Just did awesome all right. So let me quickly add you so I have now Justified my existence.

D

A

Anyway, welcome yeah I had the I had the same problem.

D

um Okay, do you still you're it's fixed now right, did you get uh let's see.

A

Double click it not. For me, it's not fiction.

D

Okay, how's this you, you, post post, your email address in the chat. I will solve the problem.

A

All right, yeah um join group. Okay, that looks like it's a different group from the one that was in the uh my email address is in the chat. um Okay,.

D

A

Group, oh there, it.

D

A

uh That group is a different one than was in the.

D

Okay, yeah I, don't I, don't know why. But uh let's uh let's, let's get you added first and.

E

A

D

Have we do need to fix up larger, larger problems, but let's.

A

See the group that Spencer shared was the functional one: okay,.

D

And you're also at sonotype right yeah.

A

Yeah um saw the presentation last week, I didn't actually realize until uh you guys presented in Tahoe. I didn't realize that there was a community meeting for score card, so I've actually got a couple commits on scorecard without ever even realizing that there's a community awesome.

D

Welcome and thank you I I do want to since you're here a quick thank you to sonotype, uh who did some really really interesting analysis this year, that showing you know the the utility of some of these metrics and just you know, and their their predictive power for at least known vulnerabilities, I, just I thought uh it was really awesome to see that kind of analysis. So thank you.

A

Yeah yeah the scorecard got a huge, huge shutout in our research paper this year.

D

Yeah Brian, just posted plus one thousand.

D

Yeah, exactly a lot of the work happens with issues and pull requests and so on, uh as you can already see so you're so, uh but okay, so who's our lead, I I'm not actually leading this meeting who's leading our meeting today, I'm.

B

Facilitating I'm.

D

B

I I'm, that that's me all.

D

Right: okay, okay, there.

A

You go, we hijacked from.

D

uh It's Laurent.

D

Sorry, isn't that oh wait! I I choose the leading the meeting. Oh.

A

I was did I, get it wrong. I think Robert was talking about.

A

D

Sorry about that, no.

B

D

Gonna Mark Mark you as the lead leading meeting, so we know.

B

Okay, um yeah so I, don't see a bunch of um project or individual updates, so yeah I can go on to the agenda and, like yeah, feel free to add things over the duration of the meeting. If anything uh comes up uh but yeah, the issue that I wanted to talk about is uh handling code review for automated or bot generated commits on a project.

B

So there are a couple of issues that have been opened around uh like uh how how we, how we score code review for uh for for, commits and uh just to give a context. uh The code review check today looks at GitHub, PR's and whether uh it could have for for any any commit in the uh kind of like default branch of the project.

B

um We we check for GitHub PR's that have been uh open corresponding to that and uh whether.

D

B

Was closed or like merged by someone other than the person who opened it um so they're like, however, we don't have a great story around um how we handle uh like comments generated by by Bots, um so like an example of a bot is like renovate or dependabot, um but people also use Bots for, like other things, that use like uh Bots that do like uh back ports or like automated, like uh automated, like, uh um if there's like a regression or a bug, they'll like automatically back out a commit.

B

So it's like, like a wide variety of bots um and uh the scoring can like be be inconsistent.

B

um Sometimes Bots are given like direct, commit access to a to a a repo uh so like they they will be in kind of like they'll show up in in the commit history as like, there's like a bunch of verified, commits from a bot, uh so uh Spencer, Laurent and I.

B

Like uh talked a little bit about this offline and there's kind of like a proposal um that want to like get people's uh thoughts on uh today, we kind of look at the last uh last 30 commits and out of those commits, we'll see how many, how many, like distinct uh change debts. There are uh meaning like how many, how many of those commits belong to like some reviewable activity like a PR um and we'll give like a proportional score.

B

So if A, Change, Is reviewed or not, we'll, like proportionally uh yeah like give like seven out of ten, if like 21 out of 30 or 30 of those commits, are like reviewed, um but our proposal is like to move this like more proportional. uh So if there's any Presence at all of blood commits that are unreviewed, uh we would just like subtract a flat uh minus three and if there's any human commits that are unreviewed, we would like uh subtract like a minus seven for that yeah.

D

B

F

D

A I'm sorry go ahead.

F

Sorry, I I wanted to give a little bit more context, because I think that's there's an issue that people from sonotype actually reported at the Tahoe meeting and I. Think that's the broader context for this problem, which is, if you look at the repository, uh which is, let's say, is a single maintainer Repository uh if, if scorecard, only catches, full requests that are from either Bots or external contributors, all of those are going to be reviewed because like unless you have enabled Auto merge on Dependable, but most of the time people still click.

F

An external contributors also are always going to have an implicit review, uh whether it's it's an lgtm, explicit or like it's just you merge and you're a different person, so that kind of leads to false positive, because all the single maintainer repositories will be seen as having code reviews, even though it's not exactly true and I. Think that's not in the spirit of this talk. This of this check. This check is like more like I, wouldn't say: Insider attacks, but it's more like.

F

Is there a community that re and they review each other's uh pull requests?

F

um So as part of this, there's I think the problem that ragya brought up, which is like Bots, uh can we basically say Bots, um you know, depend about PR or renovate.prs.

F

We consider them less risky, maybe and we don't uh score the same way, even if they are like reviewed or not reviewed, and can we also differentiate humans and, ideally between humans. We would differentiate between external contributors and maintainers, uh although we don't think that's easily possible because there's no GitHub API. So unless we do some inference where we look at who merge commits and then we say: okay, those are maintainers and then we query again GitHub apis.

F

Like some convoluted process, um we think you know I, don't think we can tackle everything in one go, but but that's the broader problem, um I'm sorry, uh David I cut you off, but I wanted to give the yeah just.

A

To just to summarize, like um the uh the thing that we were talking about in Tahoe, um I think Brian talked to you was um that there is a user who came to us and was like hey. uh Why does my? Why does my scorecard score keep going down and we looked at it and uh we saw that he was being penalized for not having any any code reviews right.

A

He was just merging directly to Maine and occasionally his score would jump up, because a dependabot would make a PR huge review and his score never should go up because he's still just a single person committing directly to Maine so depend about shouldn't buffer his score uh in the way that it had been. So that was the the context.

F

And I think something else and Spencer mentioned is that when we like it's basically the fact that it's going up and down and the there's like this instability of the check um and we we don't know if we can really tackle this all in one go unless we look at more pull requests or like a longer history um but yeah anyway, yeah, that's you're, absolutely right. Thanks for bringing that up, I'll, let other people talk. Sorry.

D

Yeah so I have a potentially different view that doesn't make it better, just potentially different, let's because I think I think this is actually an interesting conversation and clearly we need to think this through and I'm expecting there'll be some change here, because I think you're right. It does seem a little odd.

D

What's currently happening, um I'm willing to accept Bots as another creature, they just happen to be computers, so I'm happy with having a bot, creative proposed change and then the human reviews it and now you have two things, reviewing something presuming that with the exception, which I'm not sure we it's worth trying with the exception.

D

If the person who created the bot is also the reviewer I, think that's the one case where that's a little more dubious, but if a bot creates something and then it's reviewed well, presumably the bot bot thought it was a right thing to do based on its code and then the human reviewed that particular change and also thought was good um and so I think that that has some value. What worries me I'm, actually fine, with uh penalizing different verbots versus human commits proposals.

D

What worries me more is the well-known Bots not being penalized I, don't I uh I, guess we're kind of assuming that certain Bots never make mistakes. um Boy, that's kind of not been. My experience, including dependabot, can.

A

We can we segment the two concepts in the notes. Yeah I feel like I feel like because, like on one point, um it's how do we handle single, maintain like single contributor projects right? uh How do we handle that and then on? The other point is the one that you're just addressing there.

D

Yeah I, I I think all.

A

D

So, let's separate it out number of contributors or I'm sorry number of maintainers versus is something reviewed. That's what your proposing Spencer is the two TCS apart am I understanding. You correctly.

C

I think I saw that he was proposing. I was going to talk about something else. Oh.

D

Okay, all right I um yeah, that that sounds like a good tease apart. So so, let's talk about commit review.

D

um I am fine with saying if a bot or another human creates something, that's one thing, be it a human or not a human. That's created a proposed change, and now you have a second person reviewing the change to see whether or not it's actually the right thing. So you've got at least two somethings. You know somebody originally proposed and somebody else reviewed it um that doesn't strike me as crazy.

D

um Now. I do think that having multiple people is a multiple maintainers is vital if for no other reason that if the first maintainer dies, uh what happens? Oh.

A

Yeah yeah, so so the thing is like this is a really really good check, because best practices state that you have multiple people working on any project right.

D

Yeah- and that's really really hard on npm is odd. For example,.

F

So would it be worth maybe to even the code review check uh what, if we looked at, say 100 commits and we look at how many people have merged or committed or merged the commit?

F

And then, if we have a single person, we say it's very highly likely that it's a single maintenance repository and there's no code review like can we differentiate based on who commits like I, guess, maybe single maintenance we can actually infer, because if we look at enough commits, we know that it's the same person reviewing or the same person merging and if we differentiate that we make sure that that person is a is a human. We know that it's like. Can we do something like this one.

D

E

There some kind of I haven't looked at exactly how it gets installed with All-Star, but a configuration file, maybe to save who the maintainers are, because if you do it that way, new maintainers would also be penalized, and it's confusing for projects. You want to increase their score to be able to do that on, like a short-term basis.

D

I haven't checked but can't um you know obviously GitHub, for example, knows who has rights to make right, to has permission to write directly to the repo, which is what I would call a maintainer, and at that point is is that's, that's something that's publicly accessible, isn't who has permissions to write to the repo? No, no.

F

It's it's nice yeah! That's.

D

The problem: okay, that's that's what we want. Github has it, but that doesn't mean I mean we might be able to convince GitHub to give us a number of uh how many maintainers there are. That would be a Pitch to GitHub, but.

A

No I don't think that would solve the problem. That's that's at hand, though, because hey right like what what are we trying to score we're trying to score the like R code reviews happening in best practice way right, that's right! That's.

D

Well, that's that's what that's that's two of them, I I think there's actually two different measurements: are they getting reviewed and how many maintainers there are I think those are two separate points. So, let's, let's focus on the on the code review um so for the code review, we do get that information publicly.

D

um You know, basically somebody else. You know if somebody proposes it, somebody else has to then merge it right. You can. We can certainly tell that publicly um I'm, actually fine with having a lower penalty for a bot under the assumption that it's you know, although a bot could create malicious changes, um it's somewhat less likely.

D

Maybe if it's widely used yeah there's the problem is you know a bot can obviously create malicious code changes too.

F

D

F

I think that's what like I've said: I I'm, also okay, on penalizing less um just because there is a risk, but it's not as big as anyone pushing like a human doing. It.

D

I think there's a lower risk for unintentional error. Bots can can insert proposed malicious code just as well as a human can, but generally you're only going to add a bot. If you don't think it's going to be malicious and typically Bots are only implementing things that are so mechanical that they're unlikely to get it wrong.

D

B

D

Guess from that argument, I think they should be penalized, less I'm, not sure, there's no penalty, that's the only thing I'm kind of dubious about is.

A

This, the normal approach to um discussing features and changes for the scorecard I I just realized like I'm brand freaking, new and I'm, just jumping right in I love, I love. How open you guys are the conversation, but this is also uh a little on the disorganized side. I'm just curious, like what's the normal process for for like changes and features and and this this type of conversation I.

D

I think you're uh I think you're, seeing it oh.

C

D

C

Like what the future being proposed is like, if something's scoped pretty well, then like it's just all on the issue, but if there's a lot of ways of tackling it, then it usually gets brought up in these meetings.

A

Cool, it's kind of brainstorming, I, guess, yeah right.

D

Right: okay, that problem was identified, but the solution is not so obvious: fair enough. Yeah.

B

Yeah, in this case, we kind of have like two issues that like are, are about the same problem, so yeah yeah,.

A

And I didn't get um I didn't get introductions from uh the the group here so who I know um I've seen Lauren and Spencer as maintainers Roger? Are you one of the one of the code maintainers on scorecard I'm, not David.

D

I I am not, however, um I I'm not well I'm, not one of the maintainers. On the other hand, I work for the Olympics foundation and I lead the best practices badge project and the two projects are both. You know trying to figure out how all projects are doing things, so we want to try to coordinate things.

A

Yeah that makes that makes a lot of sense now I'm, just trying to I'm just trying to like figure out um what's happening in this conversation. I just realized for a second I was like you know what I'm lost.

D

Yeah, basically, the issue here is when reviews happen when proposals when changes are proposed. Please actually, maybe we'll summarize, because I maybe I'm getting this wrong. You know sometimes they're by Bots, sometimes they're by humans.

D

um Sometimes they're reviewed, sometimes they're. Not if somebody just you know just automatically accepts a bot commit without a review. uh You know that seems like a risk but much lower risk than if you just merged in arbitrary humans by review about review.

F

Yeah and also in particular, we don't want to discourage people from using Auto merge independable, because you know that's a useful feature and most of us who lgtm uh depend about or renovate about PR's. We actually don't even really review them. What we we skim through them, and sometimes maybe probably just say yes, so yeah.

D

I I will say that when I I use dependabot on on my GitHub projects, but um you know I do review the proposed changes um and I. Don't always accept them. uh I do find that just because dependabot says you should do something.

D

um No, yes technically it passes, but it doesn't understand the full context. So I do think that there should be a penalty.

A

It's a difference. It's a different type of risk right. You you've got a completely different type of risk from Auto emerging. The latest update.

D

Oh, that's right! That's right! So I, so I'm, actually agreeing with the lower penalty for an unreviewed bot commit I'm just a little hesitant about the no penalty for unreviewed documents.

E

I'm not really clear on so I understand that you'd want to not have a bot counted as a contributor, if that's, if you're, for the case, where someone's committing to Maine but otherwise I see bosses, sometimes there were sometimes they have better contributions than humans. So why? What's the point of other than that? One Edge case of someone committing to Maine and not having any other contributors, I.

A

Think that's I think that's exactly what this is for the first one is: what do we do when commits are made without a review right and then there's the uh discussion of like okay? Well, if it's a human committing without review, definitely penalize it now what if it's a bot committing without review? Oh.

E

No, no penalty.

A

Low penalty, same penalty.

D

Yeah I think it should not be same penalty simply because typically Bots are implemented on things that are mechanical and they're less likely to be wrong. They can be wrong, so I think a penalty is appropriate, but um a lesser penalty is also seems appropriate again. Lower risk not no risk, therefore, lower penalty, not no penalty.

D

Leaning towards.

F

Yeah me too I'm I'm, leaning towards lower risk and also because, if you're, a single maintainer, maybe some maintainers just want to say just Auto, merge I, don't have the time to review all this, um and if they do this and we say you're not like you like, we don't want I, don't think we want to discourage them from doing this, uh even though, if they are a single maintainer, they will not. They shouldn't pass code review so.

B

I think code review wouldn't like we would focus Less on like penalizing them, and probably the code review check would take more of an approach as saying there's like not enough review data because there aren't like any like uh yeah, there isn't like human review activity to make a decision about whether this project is like subject to human scrutiny, with review or not.

E

So, in that case, do they get zero or do they get a hundred, because.

B

I I think in so like in in the default case. We would like start everyone off at 10 and kind of like give give these penalties instead of being proportional, but in the single maintainer case. I think we would go. We would uh we all like scorecard, also has like this, like uh insufficient data result, I believe so we would like to say something like that, like there's, not enough data to make a to give a score or not.

F

B

But that's that's one possibility. We could also just say zero, like I'm I'm, not sure which, which uh which one of those is better a better option. Actually so yeah.

D

Yeah, there is actually the case where somebody does their own analysis using a token that actually cannot get GitHub access and see the underlying data.

F

But uh I'm I'm, not you mean whether he's a the person he's a maintainer yeah maintainer.

D

Runs this code and gives it a token the tool could actually determine the answer. I.

F

I think if it's a pat a pat token yes but I, don't think the GitHub token from the workflow can do.

D

It uh it won't give us that access, yeah.

F

So they have this difference between yes, but that said now that they have a granular patch token. Maybe that's maybe one way that people could use it, but but but still on the other hand, if you're on your own repository, you already know what you do right. So I think that data is more useful to Consumers than it is from uh for maintainers. So so maybe that wouldn't actually resolve the issue.

B

F

Does that make sense.

D

So so um my brain is limited. I can only really handle one thing at a time. So um if, if code is just getting accepted from the outside, without any review, um I I like that I guess proposal one you know. If you know human can it's on are getting viewed by somebody else, big penalty. If bot commits are being accepted without review, you know either the bot is just editing it directly. That's a smaller penalty. You know three and seven seem perfectly reasonable to me.

D

um I I, like that proposal number one I'm a little worried about number two um am I, but is this just me or do other people think that makes sense or or what.

F

Foreign yeah, that's fine with me.

B

Yeah that that's that's all right with me, I I'm, like a little, uh you know like every time, Like A change is made to scoring we'll like get some like level of feedback on whether that that makes sense or doesn't make sense uh based on like people's workflows. So like I'm, okay with um like maybe we should like do proposal one and like uh yeah.

C

B

More feedback, or whatever before we do yeah and.

F

So one thing that.

C

F

C

Just one thing that I, like that uh Scott has been doing in some of his changes with uh security policy and license checks is that he had a list of say, 400 projects and was able to test I guess his scoring change that he was proposing and get an idea of how on these 400 projects it would affect scores uh before the change was made. So preemptively get some sort of uh quantitative data without waiting for people to come to us saying. Why did my score go down.

D

Okay, who was doing this I'm sorry I didn't catch the name.

C

Scott uh my last name's like Heisman, or something like that uh from software engineering Institute. Oh.

D

Okay, yeah yeah yeah, um okay, he's not here on the call, but I know him: uh okay uh was doing, was running, uh I, guess uh uh draft scorecard code.

C

Yeah, like he was just running his Branch against 400, repos and sort of getting uh regression testing, I guess of.

E

C

F

Yeah, what he's done is has been really really nice, because you could, he might immediately say, with the what he's proposing would be a terrible idea or not.

D

You know what I all right, let's I I like that idea, I think we should ask Scott to do that. uh Here's a crazy thought is this something maybe we should put in the scorecards CI Pipeline on, so that we can see the changes. uh How scores change based on changes to code? Is that a little heavy.

D

Basically, could we just integrate that into our CI pipeline.

A

It would still require an opinion.

D

Well, yes, it would give answers, however, in order to help make that opinion.

B

C

B

The CI pipeline um it it sounds like it might, uh it might uh affect. Like token token usage.

D

B

Might have to like think about how this would affect, like the other, like data pipelines that like, for example, a cron infrastructure, I think uses. This would probably use like the same tokens.

E

B

D

F

So that's a good reason not to we can at least encourage uh reviewers when there's like a scoring change in a pull request to ask for that data when possible. I think Scott is the only one who has done that great work until now, so.

D

Okay, let me update my proposal, then maybe we can document how to do that, so that once there's a uh a pull request, um somebody can run.

D

You know here: here's the canonical list of 400, we typically use and you know, or how to do that.

A

What was the reason for not automating it.

D

I I don't think too many tokens getting used. We we already suck up lots of tokens.

F

Wow yeah, although I think a lot of our presupping tests now run on the with the ephemeral GitHub token, and only the branch Protection One runs with the path.

F

So it might, it might not be as bad as we think, because I think the yeah, but still still I, don't know how many I forgot. How many API calls we can make with the GitHub token uh the ephemeral one right.

A

Are you using a literal, personal access token in your CI.

B

F

Have one we have one for the branch footage.

F

Because Branch protection doesn't work without this with, without a pat token, unfortunately,.

F

B

Cool yeah, so I I think we we seem like it seems, like maybe a proposal.

B

One is like a good thing to uh maybe like put on one of these issues, just like as a summary of like what the decided like or like the approach that people uh seem to seem to like so I can like put a comment on this issue about proposal, one and then for this other uh thing of like having all reports in like uh like like having having scoring reported whenever you create a PR that affects scoring I think this would be good also like create, like a separate issue, to track.

D

And- and we could start by just asking Scott to see if we can Dock and get him to document what he's doing, and maybe you know creating a standard list, because I think it would be that that would make sense. And since he's already doing that, maybe we can get him to uh give us a hand.

E

F

So for proposal number one will it would it be also good to try to get some statistics on um Harmony out of the 30 comments that we get? How many are from typically from a bot and not a bot or at least have a distribution for single maintainers? Because then we could tell what's like the minimum number of commits that we should be looking at.

F

Otherwise, the the score is going to go up and down when you hit for requests like say when you only hit bot pull requests like you're gonna have a different score depending on what what per request you get like from from a button or the bot I, don't know if that makes sense. It.

D

Does I I, don't I, don't know it's necessarily bad if it kind of goes up and down a little bit, um I mean frankly that what happens anyway with scorecards you, you add, pin dependencies instead of your scorecard changes, and we certainly don't worry about that.

A

um We don't want it to go up when it shouldn't go up.

D

Fair enough yeah yeah.

F

But that will happen anyway right. If, if uh let's say, for example, you have one direct commit to the main branch from a human yeah, and then you have six from uh depend about that are not reviewed that are reviewed, then you're gonna get a certain score, and if the other time you have One external contributor has a commit, that is a PR that is reviewed as a human and then the same as Dependable. Then it's gonna go up and down it.

D

Will go but you look more than just a couple. What's the number now number commits You Look Backwards on.

F

Yeah, that's the problem. We look at 30, but uh the the commits that we look at might be part of the simple request and that's a an existing issue. We have to fix. So we need to decide how many pull requests we should be. Looking at. Please uh correct me: uh Spencer.

B

Thanks yeah, um particularly in case so, if you're, if you don't squash your PRS, then they kind of like a PR with like a bunch of changes like a lot of back and forth will appear as like uh 30 commits or whatever, and it could or like 50 commits, and we might not even get all of them. So might only have like one PR to look at over the history of the like. Okay,.

D

B

What yeah, within the scope of commits.

D

Okay, that's that's just an artifact of how you're counting the commits backwards.

A

Where is the code for um because I'm looking at the codereview.go uh raw I'm, looking at Raw code review um but I'm not up to speed on this check yet so I'm trying to figure out if there's Separate Checks for like?

A

Let me know what my question is: I'm. Sorry, you guys is there Separate Checks for the for the to make sure that code has been reviewed like.

A

It seems like there's two goals here: I'm trying to make sure that there's actually two checks for these goals.

B

A

B

Have a branch protection check which is kind of similar, which which says is code reviewer required for merge and code review, is more of like a historical check like what has actually happened in the history of the project. Has has code been reviewed that has gone into the default Branch so.

D

B

Protection is like what is the access policy or like standard for like committing.

A

Okay, because it seems like uh checking, pull requests to make sure it's like hey. If this is a dependent bot, bull request, we just ignore it. uh We we don't we're not going to score you based on your dependent, but like you're, not going to be scored well, based on your dependent bot uh merges in. Let me look at all the other pull requests and if it's a single, if it's like self-reviewed, then that would accomplish everything is like hey.

A

We look at all pull requests, except for the ones from the except for the one from Bots and then from those pull requests that we're looking at.

A

How many are we just do deductions based on the ones that are uh self-reviewed.

A

Maybe this is more obvious in my mind than than what I'm communicating out loud so.

B

Yeah I think what you're saying would all be part of the uh evaluation part of that check. The.

C

B

Raw would just like scrape GitHub, and then we have like a separate thing, called evaluation code review that does what you're describing like as logic. It implements that yeah.

A

Okay, I'll look at oh look at that I.

D

Mean maybe and here's the challenge, maybe what you really want to do- is figure out what the main line versus the merged branches are and track. 30 back on the main line, main branch.

C

That that's what we do, but based on how we pull the commits. It's look at the whatever the default branches and pull 30 commits from that, but you still run into the unsquashed issue.

C

D

C

D

It I mean, which is of course, I mean it's not technically wrong. It really is 30. You know, 30 commits not squash, it's just fast forwarded, but.

B

F

So I have a more high level question like uh I. Think someone brought up the branch protection I. Think right if you did so so why? Why don't? We use the branch protection settings more in the code review to save it?

F

You are doing code reviews, I mean I, guess sometimes it's using a different uh review, Bots like what is it Garrett, but I, wonder whether then for Garrett and these other ones, we actually try to count uh commits and in that sense, I think that there is no problem of squash and non-squash, because it's not full request.

F

I think maybe there's four requests but- and maybe that would allow us to be more precise on on GitHub for GitHub reviews, where we say we know you are doing uh reviews, because the branch protection settings says so and I know it can be changed but like like it depends if we say that con like maintainers are adversarial or they're. Just you know doing they are not trying to game the system or not because like if they just don't change it all the time.

F

That gives us a strong signal that we could maybe then validate with the code review uh to make sure it's it's not violated, um but maybe put more trust in the branch protection setting I'm just asking.

B

Yeah um in in my mind- and this is like a little bit separate from from this issue but like Branch protection like philosophically Branch protection is like point in time, and code review is historical, which is like Branch. Protection could be up like you could turn on brand protection.

B

30 seconds before running scorecard and scorecard would give you like full points, whereas code review is like a historical thing of all the commits in a in a project um and then, like uh the other part of what you said, which is like, uh uh can we look at like other uh review platforms? uh We we do look at other review platforms, but we apply much much less scrutiny just because we don't have like all the code to handle. What would like uh what would like an approval.

B

uh Look look like, um although we we we, we definitely should handle things like fabricator and Garrett. But right now we just kind of say, like um all those commits from other platforms are are kind of like.

C

B

Don't need reviews so like we, we just like score them. Yeah foreign.

F

Yeah I mean I'm trying to see of what the like how the data the results are consumed. So if, if I'm just trying to understand how many, how my dependencies behave, I I almost feel like Branch protection is kind of a signal that is, it can be gamed, but it's it. It gives me a an idea of uh who is doing code reviews and who is not, um and then there's the other part where oh I'm, taking the view of there's an Insider attacks uh like red team exercise and I want to catch it through.

F

You know, attestations like salsa does, for uh you know Source requirement, and that seems to be maybe something different and it looks almost like we're trying to tackle both uh we're trying to like the code review check right now is kind of trying to say I'm, verifying that I can give you an attestation and that's useful for the maintenance themselves when they want to deploy something as part of salsa but I.

F

Think for the consumers like say Google I, don't know Microsoft ingesting dependencies, so maybe the the branch protection is what they care more about than like the individual. Commit they are more looking like at the at the the practice as a trend, I guess.

D

I I think I can make a decent case for what's being done right now. You know the branch protection is a signal that hey we have an intent to from here on. Do reviews get some credit, but you know what the what's oftentimes the best way to figure out what's likely to happen in the future, is to see what's been happening in the past.

D

You know, if somebody you know, turns on and off Branch protection every time they want to do something, that's kind of lame uh watching what they actually do um has some value.

A

What, if we do it, um what if we do all commits from a certain date so like all commenced since seven days ago, something like that.

D

I think you want more than seven days, but I I get it.

D

Maybe you want both at least X number and at least this date I'm, not sure that we can actually do that in a simple query, but foreign.

F

Spencer has looked at a little bit how to do queries, I. Think right now we just say: 30 commits because graphql gives us an.

D

Easy way to express.

F

This but I don't know Spencer have you seen if we can give a timeline like since X hours.

C

Yeah, um there's sort of like two answers to this I'm sure, there's an expression we could use in graphql to sort of get that um and 30 commits was picked.

C

You know before I started looking at the code, but it's nice to have one uh query that we submit ahead of time because the the weekly Crown job that runs this on 1.2 million repos, is trying to minimize the amount of API calls yeah, um but at the same time, for someone running on like a CLI or something it's entirely possible, uh I, don't know explicitly if it's, uh if it supports, give me every commit in the last X days, uh I wouldn't be surprised if it does, um and it's possible to you know, keep listing that sort of thing uh we just have to be careful.

C

Also, now that we have uh One external contributor working on gitlab support. The this you know, functionality that we're wanting for is a generic way and right now the interface is list commits I, think and it just returns commits so just something to keep in mind.

D

Yes, I I do know that in SQL you can say you know after this date and limit. uh You know up to this number. You know you can have limit statements. I would imagine that graphql has the same. That said, I, don't know what get a get lab does it wouldn't I mean they may just have a simple paged interface, in which case you can just page backwards until you reach a date or a um you know, either a date or the limit number.

D

um If we have to do a page, that's obviously less efficient, uh but maybe that'll be compensated for with uh the fewer number of projects we have to read. So that may actually work out just fine.

D

Just get uh does git lab. What does gitlab have for the interface? I have not actually worked with their data or query.

D

C

I'm not really familiar with how it's being implemented.

D

um Okay, just on the scorecard I I, yeah, yeah I, just I just did the quick uh look up on uh get lab uh queries and I immediately suddenly find out about SQL query guidelines. So there may be a SQL interface, I I, don't know!

D

This is definitely not a thing to figure out right now, but just but if they support SQL, then they for sure have a limit statement and you can do a query and limit the max number. So it may be. Okay.

B

A

Could any of this be achieved by just looking at the git log, or what of this could not be achieved by looking at the git log.

D

You certainly can't I mean you get the same data with get log for sure, because that's.

A

Vendor agnostic.

D

It is, it is also um quite a bit of yank down before some of these get Pro for some of these projects. If you do a get clone, it's a non-trivial amount of data that you're pulling and I've done this so I that I've done and yes I know you can do that with get directly.

B

Yeah we we do look at the git log uh in the case of GitHub PRS. The way that we fetch a PR, the pr that's associated with a a a a git commit is by like using the commit object. So it has like a it like Points each commit to a Associated, PR and other review platforms.

B

Do something different like they put the like, for example, fabricator has like the differential Revision in the commit like the commit message itself, so it's like a little bit easier there to like correlate, but with GitHub we actually, we use a lot. We have one API call for like the log like another and like part of the graphql, is like to get the associated PR.

A

um If the only problem is the size of the transactions, a bare clone could just pull the history, and then you can work with it.

A

uh The git history, the git log I, don't want to be too skeptical and propose a massive change to the entire underlying code base here, but it seems like you could accomplish a lot if you're, just working with the git history and a bear clone, would let you do that.

D

Well and there's nothing that doesn't say that we can't Implement things in one way, with GitHub in a different way with something else, because there's certainly a number of you know a number of different systems out there.

B

Yeah uh we we have like a little bit more than five minutes left, so I just wanted to ask if, like uh if people want to talk about something else or have any other like topics that they.

A

B

A

To uh get a PR up with uh changing some of the checks to just look at the get log instead. Is that something you guys would be would welcome reviewing.

C

So there's an issue that has a suggestion already I'm trying to find it, but I can ping it out when I find it.

A

That'll be helpful.

B

Are you saying, like uh change code review to just look at the git lock.

C

um Just to use git log data in general, um I forget which check it was the the issue is raised with.

D

Well, that would eliminate the whole trying to limit necessarily to 30. I mean you. Could you could easily say I'm going to go back a year or say.

A

Good I think you'd have a different set of problems there uh on on. Like hey, you can never increase your score, but but yeah yeah you, you dramatically increase your right.

D

Yeah, well, you can increase your score. You're gonna, you're gonna have to show us over a year that you're gonna do it and throughout the year as you do it, your score will slowly increase that.

A

That would be true.

D

Now, before you laugh too hard, I think I can make a counter. Basically, you know we yeah. If we you want to get credit, show over a period of time that you do the work.

A

Yeah I'm, just thinking of it from the context of the security slam that that we did uh last month like nobody would get to 100. Then that would be a that'd, be a pain, that'd, be a pain.

D

Start early play the best time to plant a tree is 20 years ago right uh well,.

A

First, first task find out if this actually works. So um yeah drop me off the issue. If, uh if you find it otherwise I'll uh I'm about to have a baby, so I mean this could be all move. I might I, might just not do it, but um but I'll put up a PR with a subset uh I'll. Try to keep it small. If, uh if we can improve the viability.

B

Yeah um congrats, first of all, yeah. Congratulations.

D

um We're not gonna be able to compete with that.

B

um Yeah, there's like a I, think there's like a switch of.

C

B

Based versus file based uh uh checks within the code base, so that might be like a interesting thing to look at um it, like classifies checks by the kind of data that they need to work.

A

B

A

Where is it? Is that, like uh you're talking about a switch inside of scorecard right, yeah.

B

Within the scorecard, there's like a like, like a flag per check that that says whether it's like um oh yeah, I'd, have to stop sharing to pull it up, but.

A

B

A

Just keep I'll keep an eye out for it.

B

B

B

C

Pretty sure that's the one um but I'll have to read through it more to double check.

B

B

Is there anything else that anyone wants to talk about.

D

I think we're out of time anyway,.

B

B

uh Thanks everyone for the discussion and we'll see you all in um I believe uh two weeks is: is that our second to last meeting of the year, the first so yeah.

B

We have one more on the 15th on the calendar, but I'm not sure if that's like legitimate.

D

It's up to this group, you, you can meet on Christmas. If you want to I, won't be there.

B

I'd love that not.

D

Have me there yeah I got it.

A

Well, thanks guys, uh it was a pleasure I'm.

D

Gonna drop and thank you for the new folks welcome. Thank.

E