-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expand the ManyVersions error message #379
Expand the ManyVersions error message #379
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #379 +/- ##
=======================================
Coverage 85.36% 85.36%
=======================================
Files 18 18
Lines 2125 2125
=======================================
Hits 1814 1814
Misses 311 311
☔ View full report in Codecov by Sentry. |
gto/exceptions.py
Outdated
@@ -83,7 +83,7 @@ def __init__(self, name) -> None: | |||
|
|||
|
|||
class ManyVersions(GTOException): | |||
_message = "'{versions}' versions of artifact '{name}' found" | |||
_message = "Multiple versions ('{versions}') of artifact '{name}' found on the same git commit. Multiple versions are not allowed." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Multiple versions are not allowed on the same Git commit. Found of an artifact on
wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I am not able to understand your suggestion: "Found of an artifact on"?
Would this be ok?
Multiple versions are not allowed on the same Git commit. Violation detected for artifact '{name}', versions '{versions}'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case it is good, I pushed the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor comment. Thanks @francesco086 . Overall we can go as is. Let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@francesco086 was this addition intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Of course. Not. 😅
Sry about that
Contributes to #377