search

sourcegraph / zoekt

Fast trigram based code search
Apache License 2.0
736 stars 83 forks source link

Make IndexGitRepo signal whether there were changes #781

Closed isker closed 5 months ago

isker commented 6 months ago

As described, we'd like this information to be externally available to drive metrics for incremental indexing.

Closes #780.

cla-bot[bot] commented 6 months ago

We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

Sourcegraph teammates should refer to Accepting contributions for guidance.

isker commented 6 months ago

Hmm, I've contributed in the past without signing that.

keegancsmith commented 6 months ago

Hmm, I've contributed in the past without signing that.

I'll be honest I don't know much about these things, let me check in.

cla-bot[bot] commented 6 months ago

We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

Sourcegraph teammates should refer to Accepting contributions for guidance.

isker commented 6 months ago

Sorry about those test failures, to be honest I thought go build ./... compiled tests. Should be good now.

isker commented 5 months ago

@keegancsmith ping

jtibshirani commented 5 months ago

@isker sorry for the slow response!! We require a CLA for Zoekt contributions too. I can merge right after it's signed.

isker commented 5 months ago

@jtibshirani Please help me understand. This has not been required for my and others' previous contributions, and it of course was not required for contributions to the project as it existed before sourcegraph took ownership.

The project has in fact already seen a situation like this where the CLA requirement was determined to be a mistake (presumably because it was blanket applied to the sourcegraph org?): https://github.com/sourcegraph/zoekt/pull/409#issuecomment-1216944947. You're saying this is not a mistake this time? cc @jdorfman.

jdorfman commented 5 months ago

@isker looking into this now.

jdorfman commented 5 months ago

@isker Update: I reached out to legal to understand why your past 7 commits were CLA-free. I will follow up ASAP.

isker commented 5 months ago

My past commits were CLA-free because there was no requirement to sign a CLA, and indeed you removed that requirement after it was briefly added for a time: https://github.com/sourcegraph/zoekt/pull/409#issuecomment-1216966870. It is the same for any other past contributor to the project; it has nothing to do with me in particular.

jtibshirani commented 5 months ago

@isker I was mistaken about our policy, I apologize for that! @jdorfman thanks for jumping in here and helping figure this out.

isker commented 5 months ago

Thanks. This otherwise just needs somebody to merge 🌞.

jdorfman commented 5 months ago

@keegancsmith @jtibshirani, you can override and merge this while I deal with legal. If someone asks you why, you can link back to this very comment. :)

@isker thanks so much for your contribution. I apologize for the unpleasant experience. That's on me.