Fix typo in warning about existing volume #10623
Conversation
Previously, this was telling us "but was not created for project [project-it-was-created-for]", which is wrong. I opted to make the message super explicit and print both the actual project and the expected project. Signed-off-by: Jeremy Fleischman <jeremyfleischman@gmail.com>
jfly
force-pushed
the
jfly/tweak-warning-message
branch
from
30d6443
to
3d05a1b
Compare
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## v2 #10623 +/- ##
==========================================
- Coverage 59.57% 59.51% -0.07%
==========================================
Files 107 107
Lines 9437 9437
==========================================
- Hits 5622 5616 -6
- Misses 3239 3243 +4
- Partials 576 578 +2
☔ View full report in Codecov by Sentry. |
milas
requested review from
a team,
nicksieger,
ndeloof,
StefanScherer,
ulyssessouza and
glours
and removed request for
a team
| @@ -1194,7 +1194,7 @@ func (s *composeService) ensureVolume(ctx context.Context, volume types.VolumeCo | |||
| logrus.Warnf("volume %q already exists but was not created by Docker Compose. Use `external: true` to use an existing volume", volume.Name) | |||
| } | |||
| if ok && p != project { | |||
| logrus.Warnf("volume %q already exists but was not created for project %q. Use `external: true` to use an existing volume", volume.Name, p) | |||
| logrus.Warnf("volume %q already exists but was created for project %q (expected %q). Use `external: true` to use an existing volume", volume.Name, p, project) | |||
There was a problem hiding this comment.
@milas, I could use some help here with the code coverage complaint. I checked, and none of the existing tests seem to execute this ensureVolume function at all, so there isn't anything easy for me to copy paste to start writing a test to cover this code. I don't know much of anything about go or this codebase, so I could use some pretty hand-holdy pointers if you want me to try to do that.
There was a problem hiding this comment.
@jfly our code coverage configuration doesn't work correctly ATM, so don't worry, your PR won't be blocked by this check
Previously, this was telling us "but was not created for project [project-it-was-created-for]", which is wrong. I opted to make the message super explicit and print both the actual project and the expected project.
What I did
Related issue
I couldn't find an existing issue for this.
(not mandatory) A picture of a cute animal, if possible in relation to what you did
This surprised koala has been my phone background ever since I got my first smartphone: