Annotate and remove support auto commit #290
Conversation
…now it is not able to discard pre-existing changes.
Codecov ReportBase: 86.10% // Head: 86.52% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #290 +/- ##
==========================================
+ Coverage 86.10% 86.52% +0.41%
==========================================
Files 15 16 +1
Lines 1929 1996 +67
==========================================
+ Hits 1661 1727 +66
- Misses 268 269 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
There are two things missing yet:
|
| @@ -472,6 +479,7 @@ def annotate( | |||
| ), | |||
| label: List[str] = Option(None, "--label", help="Labels to add to artifact"), | |||
| description: str = Option("", "-d", "--description", help="Artifact description"), | |||
| auto_commit: bool = option_auto_commit, | |||
There was a problem hiding this comment.
WDYT about using just commit instead of auto_commit? For CLI and API. And push instead of auto_push. Looks like it's sufficient and shorter.
There was a problem hiding this comment.
Very good idea!
However, may I suggest for this PR to stick to auto_commit, for consistency with the pre-existing auto_push?
After this PR is merged, I will create a separate PR to rename all of them :)
I don't like mixed up PRs, as you may have noticed :P
| def remove( | ||
| repo: str = option_repo, | ||
| name: str = arg_name, | ||
| auto_commit: bool = option_auto_commit, |
There was a problem hiding this comment.
what should be the commit message? I was thinking to use the command used, something like
gto.api.annotate(repo=., name="new-type", auto_commit=True)
To your first question. Sounds ok to me. It can also be something like "Annotated artifact 'new-type' with path='path/to/it', type='type-of-it', ...".
As a small side quest we could also add --message arg (like there is for gto register), to allow writing custom commit messages.
There was a problem hiding this comment.
Good, will do that then.
Regarding the --message option though I am not 100% convinced: it adds complexity to a CLI which is already not that straightforward, and I wonder if it will ever be needed. If it is really needed in special circumstances, one can still commit manually with a git command.
I'd suggest to leave it out for now, and think of it only if some user request it. Do you agree?
There was a problem hiding this comment.
Yep, totally agree. Let's keep it out of the scope for now.
francesco086
force-pushed
the
annotate_and_remove_support_auto_commit
branch
from
7a35696 to
66c5728
Compare
…ningful commit message
… to auto commit files with dangling modifications
|
Hi @aguschin , I think it's ready, I covered the two points left. Please have a look, in couple of points I had the impression that perhaps the code was getting difficult to read. Oh, and a small thing: could you please assign this PR to me? I am not clear if this is important for the Hacktoberfest, but just in case... :P |
Co-authored-by: Alexander Guschin <1aguschin@gmail.com>
aguschin
added
hacktoberfest-accepted
|
Awesome! Merging this. Thanks @francesco086! |
Contributes to #25