CL 107296 has created a GerritCL.complete method and started
using it to enforce non-nil Meta and Commit fields in GerritCL
that are returned to the client.
It was used in ForeachCLUnsorted to skip incomplete CLs when
iterating over the gp.cls map, but due to an oversight, it
was not added to ForeachOpenCL. As a result, it was possible
for incomplete CLs to be iterated over via ForeachOpenCL.
This change fixes that by adding the missing complete check.
Update TestGerritMetaNonNil test to check ForeachOpenCL too.
Add missing period at the end of a sentence in ForeachCLUnsorted
documentation.
Fixesgolang/go#27536.
Change-Id: I453d0f2b2b2793ed1b8a55bae1c2254906d74792
Reviewed-on: https://go-review.googlesource.com/135677
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 132995 made a lot of changes to the gophers table based on
the output of gopherstats -mode=find-gerrit-gophers, which is
a new cmd/gopherstats mode added in that very CL.
The current implementation of Person.mergeIDs makes it so that
the first email seen is considered the Gerrit email.
Many of the additions of CL 132995 did not take that into account,
causing the Gerrit field to change for some gophers incorrectly.
Overall, that CL caused the following changes to gophers:
Github field changes:
tal@whatexit.org: GitHub="" -> "TomOnTime"
Carl Henrik Lunde: GitHub="" -> "chlunde"
Dieter Plaetinck: GitHub="" -> "Dieterbe"
Diogo Pinela: GitHub="" -> "dpinela"
Frank Schroeder: GitHub="" -> "magiconair"
Gregory Man: GitHub="" -> "gregory-m"
Jan Berktold: GitHub="" -> "JanBerktold"
Jean de Klerk: GitHub="" -> "jadekler"
Josselin Costanzi: GitHub="" -> "josselin-c"
Martin Garton: GitHub="MartinGarton" -> "mjgarton"
Matt Harden: GitHub="" -> "nerdatmath"
Michael Darakananda: GitHub="" -> "pongad"
Mostyn Bramley-Moore: GitHub="" -> "mostynb"
Nicholas Maniscalco: GitHub="" -> "nicholasmaniscalco"
Roland Illig: GitHub="" -> "rillig"
Yasha Bubnov: GitHub="" -> "ybubnov"
Zheng Xu: GitHub="" -> "Zheng-Xu"
ltnwgl: GitHub="" -> "gengliangwang"
oiooj: GitHub="" -> "oiooj"
thoeni: GitHub="" -> "thoeni"
Gerrit field changes:
Andrew Bonventre: Gerrit="andybons@golang.org" -> "365204+andybons@users.noreply.github.com"
Carl Mastrangelo: Gerrit="notcarl@google.com" -> "carl.mastrangelo@gmail.com"
Chris McGee: Gerrit="sirnewton_01@yahoo.ca" -> "newton688@gmail.com"
Eric Lagergren: Gerrit="ericscottlagergren@gmail.com" -> "eric@ericlagergren.com"
Filippo Valsorda: Gerrit="filippo@golang.org" -> "6195@62eb7196-b449-3ce5-99f1-c037f21e1705"
Guillaume J. Charmes: Gerrit="guillaume@charmes.net" -> "gcharmes@magicleap.com"
Harshavardhana: Gerrit="hrshvardhana@gmail.com" -> "harsha@minio.io"
Jean de Klerk: Gerrit="jadekler@gmail.com" -> "deklerk@google.com"
Joe Tsai: Gerrit="joetsai@google.com" -> "joetsai@digital-static.net"
Martin Möhrmann: Gerrit="moehrmann@google.com" -> "martisch@uos.de"
Matthew Dempsky: Gerrit="mdempsky@google.com" -> "matthew@dempsky.org"
Olivier Poitrey: Gerrit="rs@dailymotion.com" -> "rs@netflix.com"
Paul Jolly: Gerrit="paul@myitcv.org.uk" -> "paul@myitcv.io"
Ralph Corderoy: Gerrit="ralph@inputplus.co.uk" -> "ralph.corderoy@gmail.com"
Raul Silvera: Gerrit="rsilvera@google.com" -> "rauls5382@gmail.com"
Richard Miller: Gerrit="miller.research@gmail.com" -> "millerresearch@gmail.com"
Sebastien Binet: Gerrit="seb.binet@gmail.com" -> "binet@cern.ch"
Tobias Klauser: Gerrit="tobias.klauser@gmail.com" -> "tklauser@distanz.ch"
Vitor De Mario: Gerrit="vitordemario@gmail.com" -> "vitor.demario@mendelics.com.br"
Googler field changes:
Aaron Kemp: Googler=false -> true
Jason Hall: Googler=false -> true
Jean de Klerk: Googler=false -> true
(It also caused many emails to be added, but I'm not considering
those changes since they're not relevant to golang/go#27517 and
aren't causing harm.)
All of the Github and Googler field changes are good,
but not all of the Gerrit field changes are good.
I've manually checked them against the
go-review.googlesource.com Gerrit server,
and classified them as follows:
bad Andrew Bonventre: Gerrit="andybons@golang.org" -> "365204+andybons@users.noreply.github.com"
bad Carl Mastrangelo: Gerrit="notcarl@google.com" -> "carl.mastrangelo@gmail.com"
ok Chris McGee: Gerrit="sirnewton_01@yahoo.ca" -> "newton688@gmail.com"
bad Eric Lagergren: Gerrit="ericscottlagergren@gmail.com" -> "eric@ericlagergren.com"
bad Filippo Valsorda: Gerrit="filippo@golang.org" -> "6195@62eb7196-b449-3ce5-99f1-c037f21e1705"
bad Guillaume J. Charmes: Gerrit="guillaume@charmes.net" -> "gcharmes@magicleap.com"
bad Harshavardhana: Gerrit="hrshvardhana@gmail.com" -> "harsha@minio.io"
ok Jean de Klerk: Gerrit="jadekler@gmail.com" -> "deklerk@google.com"
bad Joe Tsai: Gerrit="joetsai@google.com" -> "joetsai@digital-static.net"
bad Martin Möhrmann: Gerrit="moehrmann@google.com" -> "martisch@uos.de"
bad Matthew Dempsky: Gerrit="mdempsky@google.com" -> "matthew@dempsky.org"
ok Olivier Poitrey: Gerrit="rs@dailymotion.com" -> "rs@netflix.com"
bad Paul Jolly: Gerrit="paul@myitcv.org.uk" -> "paul@myitcv.io"
bad Ralph Corderoy: Gerrit="ralph@inputplus.co.uk" -> "ralph.corderoy@gmail.com"
bad Raul Silvera: Gerrit="rsilvera@google.com" -> "rauls5382@gmail.com"
ok Richard Miller: Gerrit="miller.research@gmail.com" -> "millerresearch@gmail.com"
bad Sebastien Binet: Gerrit="seb.binet@gmail.com" -> "binet@cern.ch"
bad Tobias Klauser: Gerrit="tobias.klauser@gmail.com" -> "tklauser@distanz.ch"
bad Vitor De Mario: Gerrit="vitordemario@gmail.com" -> "vitor.demario@mendelics.com.br"
I considered any @google.com -> non-@google.com changes as bad.
For the rest, it was based on which email was recognized by the
Gerrit server and had more activity overall, as well as recently.
This CL undoes all the bad Gerrit field changes, reverting them
to their original pre-CL 132995 values. It also changes the Gerrit
email for Gopherbot, and cleans up my own entry. That leaves just:
Gerrit field changes:
Chris McGee: Gerrit="sirnewton_01@yahoo.ca" -> "newton688@gmail.com"
Jean de Klerk: Gerrit="jadekler@gmail.com" -> "deklerk@google.com"
Olivier Poitrey: Gerrit="rs@dailymotion.com" -> "rs@netflix.com"
Richard Miller: Gerrit="miller.research@gmail.com" -> "millerresearch@gmail.com"
In future CLs, we'll need to be careful with the order in which
emails are added, until golang/go#27631 is resolved.
Updates golang/go#27631.
Fixesgolang/go#27517.
Change-Id: I6bd289af6ea2c50c293c4576de3873658994b98a
Reviewed-on: https://go-review.googlesource.com/135456
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Gerrit has changed the format it uses for the Change URL in its output.
It used to be something like:
remote: SUCCESS
remote:
remote: New Changes:
remote: https://go-review.googlesource.com/#/c/dl/+/134117 remove blank line from codereview.cfg
Now it is:
remote: SUCCESS
remote:
remote: New Changes:
remote: https://go-review.googlesource.com/c/dl/+/134117 remove blank line from codereview.cfg
(The '#' path element in the Change URL is removed.)
Update the regexp to match, and add a test for it.
This fix will likely cause the previously unposted "This PR has been
imported to Gerrit for code review" comments to get posted during
next pull request activity, since they haven't already been.
Fixesgolang/go#27561.
Updates golang/go#27504.
Change-Id: I37a75ddb8d879017ebd887c651ca74a6ad6b892d
Reviewed-on: https://go-review.googlesource.com/134175
Reviewed-by: Andrew Bonventre <andybons@golang.org>
In golang/go#23223, we've created a new subrepository:
https://go.googlesource.com/dl
Unlike other subrepositories, there isn't a mirror on GitHub yet:
https://github.com/golang/dl (404 at this moment)
We want to include it in the whitelist for mirroring to GitHub,
including mirroring its pull requests to Gerrit. The motivation is to
make this subrepository consistent with all others and prevent people
from running into an unexpected situation when some repos are mirrored
but some aren't.
We don't actually expect much contribution activity to the dl subrepo,
because it's very narrow in scope and mostly generated code.
Updates golang/go#26949.
Change-Id: Id995bb3d97b8eb328e6c6bba6714bc74fbefb942
Reviewed-on: https://go-review.googlesource.com/133896
Reviewed-by: Andrew Bonventre <andybons@golang.org>
This is a starting point for adding an aix-ppc64 reverse builder.
There can be further adjustments made to it as needed.
Updates golang/go#27160.
Change-Id: I8a61adbe21f1a08265df9a206d94bb04e880a508
Reviewed-on: https://go-review.googlesource.com/131335
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The coordinator used to run on GCE and for some reason (graceful
upgrades, freeing up memory leaks?), the process would exit after it
had been up for 10 minutes and had no work to do.
Delete that. We run on Kubernetes now and don't need graceful upgrades
and shouldn't be leaking resources. (If we do, we should be fixing
that instead.)
Also it appears that this code no longer does anything.
Change-Id: I9b5f2881692117b7c99dd2e513ee146f43170791
Reviewed-on: https://go-review.googlesource.com/132075
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
It's perf time at Google, so it's time to answer the ol' "What Would
You Say You Do Here?" question.
So, time to update gopherstats with more stats & update the gophers
package in the process.
Change-Id: I0e0981f40a1229f943130621bd55acedffc7a583
Reviewed-on: https://go-review.googlesource.com/132995
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Look for changes that haven’t had any human aside from the author
on them and assign reviewers/CCs based on entries in golang.org/s/owners.
If no owners can be found, the tag 'no-owners' is added to the
change to prevent gopherbot from making unneeded API calls.
Also updates gopherbot's Dockerfile to use 1.11 and update deps.
Change-Id: I2650a10dd324532d86bc902be419c5f29ae980db
Reviewed-on: https://go-review.googlesource.com/121018
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Done more stuff there recently and it needs attention, so it seems like
a right fit.
Change-Id: I6344ccc5da83c0cfecd7a7f2ecc1399497e0de03
Reviewed-on: https://go-review.googlesource.com/131285
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This removes the tour from the release binary for go1.12.x and later
releases, lowering the size of each release. It will now redirect to
tour.golang.org, unless the user runs "go get -u golang.org/x/tour" to
cache it locally, in which case the local copy will be preferred.
The release size was 170.0 MiB for go1.11rc1, this change reduces the
size by 6.3 MiB, a 3.7% reduction. After this change, and others which
reduced the release size, the release candidate is now 115.2 MiB.
Updates golang/go#27151
Change-Id: I6687c34f3d4ae161c5e6df1f7af8cf3adc016fc4
Reviewed-on: https://go-review.googlesource.com/131156
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change gives gopherbot support to backport to the next major
release (current release + 1). Previously, attempting a backport to
a future release would create backports for the last two major
Go releases. These changes will not affected "please backport" comments
which do not indicate a backport number, as gopherbot should not open
an issue for the future release without being explicitly asked to do so.
Fixesgolang/go#27040
Change-Id: I0096680c2ce2e164e6d80203bcf257dadaa97138
Reviewed-on: https://go-review.googlesource.com/131077
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This removes the blog from the release binary, greatly lowering the size
of each release. It will now redirect to blog.golang.org, unless the user
runs "go get -u golang.org/x/blog" to cache it locally, in which case the
release-shipped godoc will prefer the local copy of the blog.
Currently, the blog is growing at ~2MB a year, thus including the blog in
every new go version release will continue to grow the releases over time.
The release size was 170MB for go1.11rc1, this change reduces the size
by 34MB, a 20% reduction.
Fixesgolang/go#21917
Change-Id: I53bf0c416c2085da10b6bb70f8aa6cae8e826dc3
Reviewed-on: https://go-review.googlesource.com/130575
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The SNI challenge is not supported by Let's Encrypt anymore, replaced by
the ALPN one, which requires an extra config entry.
Also, autocert now knows how to do RSA fallback, so remove that code.
Updates golang/go#27127
Change-Id: I45f907101a7c7a57d1a8376208dba4afb10ed6fd
Reviewed-on: https://go-review.googlesource.com/130418
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
plan9-386-v6 is a new plan9-386 image built with the change
done in CL 129098.
I tested it with cmd/debugnewvm on latest Go master commit,
and all stdlib tests passed, including TestVariousDeadlines.
(cmd/... tests didn't finish in time before cmd/debugnewvm
timed out.) Running it 3 more times, there was still one
TestVariousDeadlines failure and 2 successes, so we'll have
to keep watching it.
Updates golang/go#26945.
Change-Id: Ibd1e990e60c3a69a5fddede18d7a121fa319e79d
Reviewed-on: https://go-review.googlesource.com/130137
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TestVariousDeadlines is regularly failing on
the plan9/386 builder running on GCE.
Richard Miller suspects that the failure
may be related to an issue in time measurement,
due to the clock going backward and forward.
A work around would be to disable auth/timesync
to prevent prevent the time from jumping around.
This changes fixes the Plan 9 image by disabling
aux/timesync before starting the buildlet.
Fixesgolang/go#26945.
Change-Id: Ibe80cdcd1360cd9e40da70c2c454b6ba2eb44ce9
Reviewed-on: https://go-review.googlesource.com/129098
Reviewed-by: Richard Miller <millerresearch@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Check when getting an oauth2.TokenSource, and when using it.
Change-Id: I1b240e5eafc23202d7d22d74d3f5cf9a59ca292e
Reviewed-on: https://go-review.googlesource.com/130315
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
"gcloud auth application-default login" is NOT the same as
"gcloud auth login"
... which is super confusing.
Add a check for users.
We should probably add this in more places over time. Baby steps.
Change-Id: I4584f5b84b941fa6893eccd09fd61e8850538607
Reviewed-on: https://go-review.googlesource.com/130195
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
The ./make.bash command to create the image was missing, making the
text hard to understand.
Also modify the gcloud compute command example to use plan9-386-v6
as the VM image name, so that the example below makes more sense:
> Then update x/build/dashboard/builders.go with the new image version
> (e.g. "plan9-386-v5" to "plan9-386-v6").
Change-Id: I8c00ce4f255cb2dbff090539b3aafc99ffba2c4b
Reviewed-on: https://go-review.googlesource.com/129995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This saves a bunch of space, and users don't typically rebuild
cmd/compile, cmd/link, etc. If they want to, they still can, but
they'll have to pay the cost of rebuilding dependent libaries. No need
to ship them just in case.
On linux-amd64, this reduces the tar.gz size from 177,294,517 to
156,487,264 bytes, a 19.8 MiB, 12% reduction.
Change-Id: I784dcf2c6f15f8eee8b9eaa501a8d982b2627288
Reviewed-on: https://go-review.googlesource.com/129955
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Only runs manually. We can run this whenever the tree reopens.
Change-Id: I003a8b69fd212ec3040a26855796ba997f1a5943
Reviewed-on: https://go-review.googlesource.com/129817
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
The coordinator does it automatically these days.
Mention debugnewvm instead.
Change-Id: Ia3831ada77a89b1c763bc52a7723e586b7c7c024
Reviewed-on: https://go-review.googlesource.com/129915
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
It's better to authenticate with an identity (e.g. foo@google.com),
rather than all of us copy/pasting a service account JSON (a glorified
access token) around and the server not knowing who's who.
I was previously misled into using service accounts because the gcloud
tool (when run on a GCE VM) strongly urges you not to use user
accounts and says you should be using service accounts instead. But
turns out that's because it assumes you'd never use GCE for
development and only for deployment. That is, gcloud assumes you'd
only use gcloud on desktop/laptop, and then deploy your binary to GCE
where the application itself would use service accounts. (We do use
service accounts for the application, but I also develop on a GCE VM.)
Also, the precursor helper function to FindDefaultCredentials (for
"Application Default Credentials") had a different search order for
credentials and prefered the GCE VM's service account instead of the
user-specific credentials. Now that FindDefaultCredentials uses a good
order we can remove some of our old complexity.
Change-Id: Ia888e264cfb88e977f3ff1a3a4bb583db70466ab
Reviewed-on: https://go-review.googlesource.com/129416
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
When running in GCE's Container-Optimized OS (COS), we can't use
port 22, as the system's sshd is already using it. Our container
runs in the system network namespace, not isolated as is typical
in Docker or Kubernetes. So use port 2200 instead.
Remove an unnecessary type conversion.
Updates golang/go#26969.
Change-Id: Ic85e1f14529175106b9c7397186d3e9b5cb39c1c
Reviewed-on: https://go-review.googlesource.com/129356
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Empirically, the maintner client uses just under 2 GB RAM while running.
However, during a maintner.ErrSplit situation, the RAM need spikes
beyond 2 GB, causing the apps to crash due to a hard limit of 2 GB RAM.
Increasing it to 4 GB should let them run crash-free during net splits.
Change-Id: I79deec74c0ddac4afb9e2ffb2ab19747a06bd53a
Reviewed-on: https://go-review.googlesource.com/128361
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
I computed these using a build of the Go toolchain from a working copy at
https://golang.org/cl/128136, patchset 12.
The versions selected by 'go mod tidy' needed two tweaks to pass tests:
go get golang.org/x/text@master
go get github.com/russross/blackfriday@v1
The x/text version bump is because the last tagged version (v0.3.0) is very old
(last December).
The blackfriday bump is because the repository made a breaking change (v2.0.0)
without changing the import path.
With those changes, 'go test ./...' passes at the repo root.
('go test all' fails, possibly due to golang/go#26279.)
Updates golang/go#26279.
Updates golang/go#26872.
Change-Id: Ieac5327e4bddd2b78b981d7683beb98608708a3a
Reviewed-on: https://go-review.googlesource.com/128636
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When parsing commit messages for references to GitHub issues,
parseGithubRefs used to add entries to GerritCL.GitHubIssueRefs
slice whenever there was a reference, even if that reference has
already appeared before.
The GitHubIssueRef type only contains minimal information about
a reference: the repository and the issue number. As a result,
there is very little to gain from tracking multiple references
separately. Most users of the API would likely prefer a list
of unique issue references than the raw list. So, change it to
return unique issue references only, and document that.
If in the future there is a need to track issue references
at a more granular level, where it matters whether there was
one or multiple references, the API can be revisited then.
Fixesgolang/go#26113.
Change-Id: Ib51372736bcf49e3eae4cf111d0258733c742e61
Reviewed-on: https://go-review.googlesource.com/128119
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change simplifies and cleans up finishProcessingCL. It documents
a precondition for finishProcessingCL that cl.Meta must be non-nil.
finishProcessingCL is only ever called on CLs that have been noted as
dirty via noteDirtyCL, and CLs are always given a non-nil Meta before
noteDirtyCL is called on them.
foreachCommitParent sounded like it called f on all parents of commit,
not including commit itself. In reality, it called f on commit as well.
Rename it to foreachCommit and improve its documentation to make this
more visible.
As a result, it becomes more clear that cl.Metas will contain at least
one element (e.g., at least mostRecentMetaCommit). Then, cl.Metas[0]
can be used to find the CL creation time.
Also change foreachCommit to return an error if a parent commit isn't
found. Otherwise, we'd be cutting the history walk short and the caller
wouldn't know. This shouldn't happen, but if it does, it'll be logged
(by finishProcessingCL).
Define a CL to be complete if its Meta and Commit fields are non-nil,
and the Metas slice contains at least 1 element. Use this to simplify
OwnerName and OwnerID methods and remove the no longer needed
firstMetaCommit method.
Finally, use the simplified finishProcessingCL to set cl.Created to
the actual creation time of the CL (rather than "last updated" time).
It's easy now, because we can deduce that len(cl.Metas) >= 1 is
guaranteed to be true, and therefore cl.Metas[0] will not panic.
There's no need to add guards or think about what to do if cl.Metas
is empty.
Document GerritCL.Created field, since it's exported.
Background
This change was started with the goal of fixing golang/go#24744.
It was hard to fix that bug with a short diff, because there were
too few guarantees inside finishProcessingCL that could be relied on.
I wanted to improve that as part of my fix.
We already established some guarantees for the user-facing CLs (i.e.,
that their Commit, Meta fields are non-nil and len(cl.Metas) >= 1).
I started propagating and using that property in a few more places
in internal code.
It allowed simplifications such as:
// OwnerName returns the name of the CL’s owner or an empty string on error.
func (cl *GerritCL) OwnerName() string {
- m := cl.firstMetaCommit()
- if m == nil {
- return ""
- }
- return m.Author.Name()
+ if !cl.complete() {
+ return ""
+ }
+ return cl.Metas[0].Commit.Author.Name()
}
-
-func (cl *GerritCL) firstMetaCommit() *GitCommit {
- m := cl.Meta
- if m == nil { // TODO: Can this actually happen, besides in one of the contrived tests? Remove?
- return nil
- }
- c := m.Commit
- for c != nil && len(c.Parents) > 0 {
- c = c.Parents[0] // Meta commits don’t have more than one parent.
- }
- return c
-}
// complete reports whether cl is complete.
// A CL is considered complete if its Meta and Commit fields are non-nil,
// and the Metas slice contains at least 1 element.
func (cl *GerritCL) complete() bool {
return cl.Meta != nil &&
len(cl.Metas) >= 1 &&
cl.Commit != nil
}
Also, previously, it was hard to be sure that the
gc, ok := ... line wouldn't panic:
// called with Corpus.mu Locked
func (gp *GerritProject) finishProcessingCL(cl *GerritCL) {
// What happens if cl.Meta is nil?
// Should there be a if cl.Meta == nil guard here?
// What should it do if cl.Meta is indeed nil?
gc, ok := c.gitCommit[cl.Meta.Commit.Hash]
if !ok {
log.Printf("WARNING: GerritProject(%q).finishProcessingCL failed to find CL %v hash %s",
gp.ServerSlashProject(), cl.Number, cl.Meta.Commit.Hash)
return
}
By documenting that cl.Meta must be non-nil as a precondition of
finishProcessingCL, it became possible to know it won't panic:
// finishProcessingCL fixes up invariants before the cl can be returned back to the user.
// cl.Meta must be non-nil.
//
// called with Corpus.mu Locked
func (gp *GerritProject) finishProcessingCL(cl *GerritCL) {
// cl.Meta can't be nil due to precondition.
gc, ok := c.gitCommit[cl.Meta.Commit.Hash]
if !ok {
log.Printf("WARNING: GerritProject(%q).finishProcessingCL failed to find CL %v hash %s",
gp.ServerSlashProject(), cl.Number, cl.Meta.Commit.Hash)
return
}
(It would be a bug to call finishProcessingCL with a cl where
cl.Meta is nil, because it'd be violating the precondition.)
I think stating preconditions and propagating more internal invariants
is a good general direction, because it allows the code to be simpler
and easier to verify as being correct. Otherwise, one needs to keep
checking if fields are non-nil everywhere before using them and think
what to do if they are nil, and that's unpleasant.
Fixesgolang/go#24744.
Change-Id: I07e362d52e30089a9ba03c30b04ad19b2c385722
Reviewed-on: https://go-review.googlesource.com/111877
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When git has no http cookies, the request for http cookies
will fail because git will exit(1). Ignore this failure
because the output is properly tested either way. This allows
authentication to fallthrough to the netrc lookup.
Correct the netrc lookup under windows. git reads the netrc
file as "_netrc" in the users home directory.
Add a warning at the end of the function that no authentication
was set.
Fixesgolang/go#26782
Change-Id: I0ba94ff7fa4b6038d6117156dcc729ddf4616fdc
Reviewed-on: https://go-review.googlesource.com/127855
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This will allow for zero-downtime deployments, which will be
important for when services start relying on the /owners endpoint.
Change-Id: I4671dbbbf473ba07dbab585222d12832c3193fac
Reviewed-on: https://go-review.googlesource.com/127036
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Dmitri Shuralyov
Than McIntosh
Tobias Klauser
Filippo Valsorda
Brad Fitzpatrick
Greg Steuck
Andrew Bonventre
Kevin Burke
Daniel Martí
Katie Hockman
David du Colombier
He Liu
Bryan C. Mills
Daniel Theophanes