Skip to content

ref: deprecate digest option from HTTP remote #2802

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

Merged
merged 5 commits into from
Oct 4, 2021
Merged

Conversation

isidentical
Copy link
Contributor

We have removed the digest option in iterative/dvc#6525

@shcheklein shcheklein temporarily deployed to dvc-org-remove-digest-o-frx9rk September 7, 2021 10:14 Inactive
Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not remove it, but just clarify that it has been removed and put a link to a ticket in dvc repo, so people could give us feedback.

@jorgeorpinel
Copy link
Contributor

Agree to leave it for now with a note on which version it was removed but do we need to link to some GH ticket? I'd prefer to avoid that. Users can reach us via regular channels with feedback. Or is there a special reason we want to encourage feedback on this? Thanks

@efiop
Copy link
Contributor

efiop commented Sep 10, 2021

@jorgeorpinel Just to have 1 place to point people to.

@jorgeorpinel jorgeorpinel self-assigned this Oct 1, 2021
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-remove-digest-o-frx9rk October 1, 2021 18:21 Inactive
@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-remove-digest-o-frx9rk October 1, 2021 18:22 Inactive
@jorgeorpinel jorgeorpinel requested a review from efiop October 1, 2021 18:23
jorgeorpinel
jorgeorpinel previously approved these changes Oct 1, 2021
@jorgeorpinel jorgeorpinel requested review from efiop and removed request for efiop October 1, 2021 18:23
@jorgeorpinel jorgeorpinel changed the title http: remove digest option ref: deprecate digest option from HTTP remote Oct 1, 2021
@shcheklein
Copy link
Member

Was it deprecated or removed though?

@efiop
Copy link
Contributor

efiop commented Oct 1, 2021

The difference is vague. I've suggested to include a link above, but that might be excessive. The current approach will work fine for now.

@shcheklein
Copy link
Member

Agreed on the link. It's more of a question - keep deprecated term or use remove. Even from the conversation you both have here:

but just clarify that it has been _removed_
which version it was removed
We have removed the digest option

it seems that it was removed? So, why don't explicitly say removed in ...

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-remove-digest-o-frx9rk October 4, 2021 22:09 Inactive
@jorgeorpinel jorgeorpinel dismissed their stale review October 4, 2021 22:12

Passing back to core team.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-remove-digest-o-frx9rk October 4, 2021 22:13 Inactive
@jorgeorpinel
Copy link
Contributor

I changed to "removed".

I don't like the idea of linking to GH issues in general (seems fragile; we already have a few which are probably no longer relevant) but feel free to add the link to the appropriate ticket (I'm not sure which one @efiop referred to) and merge @isidentical.

@efiop efiop merged commit 82c46b4 into master Oct 4, 2021
@efiop efiop deleted the remove-digest-option branch October 4, 2021 22:32
@jorgeorpinel jorgeorpinel self-assigned this Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants