Tensorboard: Remove cla/google as a required check.

[chihuahua]

We've encountered various issues in which the check gets stuck such as
https://github.com/tensorflow/tensorboard/pull/436

and it seems very hard to determine what is at the root of those problems. How about we just remove cla/google as a required check (It'll still run, but it won't be required for merging.). @willnorris recommends that we make that check not required.

@wchargin @dandelionmane

[wchargin]

I support this, because it's a small team and googlebot has blocked us from merging PRs for extended periods multiple times.

However, it's worth noting that tensorflow/tensorflow has googlebot as a required check. Perhaps it would be worth talking to some people who run the zoo over there to see what their approaches to this problem are.

[willnorris]

We actually have a new mechanism by which a Googler can manually override the CLA check if they know for sure everything is okay. We don't yet have a nice UI for it yet, and it may not be fully plumbed through to production, but it's not far off. Googlers can read more at http://go/cla#force-approve, and you can talk to @cflewis to track progress or with questions.

[chihuahua]

Hi @nealwu and @jhseu,

As @wchargin noted, TensorFlow requires cla/google and, of course, fields many outside contributions. How do you currently deal with cases in which cla/google gets stuck as noted above?

We can squash commits with certain authors, but that unfortunately removes their history of commits.

[cflewis]

FYI the Googler override won't be available until Monday because of a bug that
@willnorris fortunately found.

I think the override will solve this in the general case and certainly that of #436, but I don't know about the others that @wchargin notes... and @wchargin himself wouldn't be able to use it, so you're back at the "ping someone for help" which is what is stopping #436 being merged right now (as you need an admin to turn the check off and do the merge). So perhaps it won't be as effective a fix as you need.

So I guess I am not saying whether you should or should not do this, but do remember CLAs always need to be OK for merging! 🤣

[willnorris]

@chihuahua please, please do not squash commits or otherwise alter the true authorship of commits in order to get around CLA issues. Instructing a contributor to use an appropriate email address is fine, but hiding the authorship of a commit is not. If you are particularly stuck on the CLA for a given pull request, please talk to us and we'll help you figure it out.

[nealwu]

Chi: For the pull request you linked to, it looks like the original issue was either that JGSweets used a different email which wasn't recognized for some of the commits or that pbanavara (who made two commits) hadn't signed the CLA yet.

Once you get to the "So there's good news and bad news" message from @googlebot, you're good to merge (though you should verify what it tells you to before merging). It will always give you that message if there are multiple commit authors.

[wchargin]

@nealwu We had verified that all emails signed the CLA. Once we got to the "so there's good news and bad news" state, we _couldn't_ merge, because Googlebot failed to report a status, so the required check was blocking. Usually it gives that message and approves the check; sometimes it just fails.

We're wondering what you do at tensorflow/tensorflow when you run into the same googlebot bug.

[jart]

Following policy both in spirit and in print is a priority to me. I'm glad a tool exists that helps us do it, even if it only works 95% of the time. All we need is good diplomacy, to keep our contributors happy in the cases where the red tape gets in the way.

@willnorris I'm assuming it would be OK to turn off the bot, merge, and turn it back on, IF we manually pull the PR branch to our machine, run git log, and check each and every single email address with the signcla/ website? Because if that's the case, then we can probably close this issue out today and not have to wait until Monday to unblock the PR in question.

Also I agree when it comes to squashing. I would even go a step further and say to never squash a PR that has multiple authors, and instead ask the contributor to git rebase -i, make the history more presentable, force push, and then we rebase merge. I think this is a good policy: not just to comply with legal, but so each individual gets credit for his/her contributions.

[wchargin]

never squash a PR that has multiple authors

I understand your goals in saying this. On the other hand, it has the effect that you have states in your history where the codebase simply does not work. This is bad: for instance, it breaks git bisect. If it is possible to separate the code so that each author's work is in a single commit _and_ the repository is correct at each intermediate state, then I'm not opposed to rebasing—but it's worth noting that in that case each commit could simply be made into a separate PR.

[willnorris]

I'm assuming it would be OK to turn off the bot, merge, and turn it back on, IF we manually pull the PR branch to our machine, run git log, and check each and every single email address with the signcla/ website

Yes. To quote from https://opensource.google.com/docs/cla/#verify

At the end of the day, we always rely on the project owners to verify the CLA status, whether that means simply looking for the commit status set by SignCLA, or by manually checking the CLA themselves. It’s okay to accept a contribution that you are certain is covered by a CLA, even if the automatic verification failed for some reason.

We're working to improve our tools so that you don't have to handle these corner cases manually, but it is always okay to do so. We're trusting in the product teams to make the right call and pull us in if they are ever in doubt.

[chihuahua]

Thanks to @cflewis and everyone's efforts, #436 got merged.

However, this problem will resurface (ie, one day, 2+ authors decide to code a plugin). What's our policy?

Making cla/google optional for merging seems to nicely balance mandating CLAs for contributors vs enabling us to unstuck ourselves.

[cflewis]

The policy is that you can do the force approve from Googlebot yourself. We have documentation internally to tell you how, ping me if you can't find it. There's no whitelisting step you need to take, just being a Googler is enough.

We trust your best judgement, so if the CLAs look good to you, you are free to force approve even if Googlebot isn't saying yes. #436 would go much quicker now this new functionality is in (note that even the open source team didn't have this force approve lever until now so we weren't intentionally blocking you, it's just lucky we got round to doing it when we did!)