RF: push since='' -> push --since='^'
#4617
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implements the decision made in dataladgh-4582 to use this character as a more intuitive alternative than an empty string. Fixes dataladgh-4582
I can replicate that locally. Real bug, stupid, and should be fixed now. |
Otherwise we would require managed branches on either end.
Codecov Report
@@ Coverage Diff @@
## master #4617 +/- ##
==========================================
+ Coverage 89.53% 89.55% +0.01%
==========================================
Files 286 286
Lines 38822 38822
==========================================
+ Hits 34760 34767 +7
+ Misses 4062 4055 -7
Continue to review full report at Codecov.
|
bpoldrack
approved these changes
Jun 9, 2020
yarikoptic
approved these changes
Jun 10, 2020
| @@ -114,8 +114,8 @@ class Push(Interface): | |||
| constraints=EnsureStr() | EnsureNone(), | |||
| doc="""specifies commit-ish (tag, shasum, etc.) from which to look for | |||
| changes to decide whether pushing is necessary. | |||
| If an empty string is given, the last state of the current branch | ||
| at the sibling is taken as a starting point."""), | ||
| If '^' is given, the last state of the current branch at the sibling | ||
| is taken as a starting point."""), |
There was a problem hiding this comment.
starting point for what? Unrelated to the intent of this PR, so I will just approve as is, but this option might need further explanation of "its purpose in life" - that unlike git " Using this option allows to avoid considering subdatasets where no changes were made since the that starting point" or alike
| # figure out state of remote branch and set `since` | ||
| since = _get_corresponding_remote_state(ds_repo, to) | ||
| if not since: | ||
| lgr.info( | ||
| "No tracked remote for active branch, " | ||
| "detection of last pushed state not in effect.") | ||
| elif since: | ||
| # will blow with ValueError if unusable | ||
| ds_repo.get_hexsha(since) |
There was a problem hiding this comment.
Thanks to the comment this suggestion
Suggested change
| ds_repo.get_hexsha(since) | |
| since = ds_repo.get_hexsha(since) |
is probably (who knows what helper functions do/report) is necessary. Just a note to myself then
There was a problem hiding this comment.
This would turn a potentially symbolic ref into a hexsha and make subsequent log messages harder to decode. I'd rather not do that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Implements the decision made in gh-4582 to use this character as a more
intuitive alternative than an empty string.
Fixes gh-4582