Skip to content

new remote rename command#1318

Merged
jorgeorpinel merged 3 commits into
treeverse:masterfrom
karajan1001:fix_1301
May 24, 2020
Merged

new remote rename command#1318
jorgeorpinel merged 3 commits into
treeverse:masterfrom
karajan1001:fix_1301

Conversation

@karajan1001
Copy link
Copy Markdown
Contributor

@karajan1001 karajan1001 commented May 19, 2020

fix #1301 (match treeverse/dvc/pull/3794)

You may disregard these recommendations if you used the Edit on GitHub button from dvc.org to improve a doc in place.

❗ Please read the guidelines in the Contributing to the Documentation list if you make any substantial changes to the documentation or JS engine.

🐛 Please make sure to mention Fix #issue (if applicable) in the description of the PR. This causes GitHub to close it automatically when the PR is merged.

Please choose to allow us to edit your branch when creating the PR.

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

@jorgeorpinel jorgeorpinel changed the title Draft of the new remote rename command [WIP] new remote rename command May 19, 2020
@karajan1001
Copy link
Copy Markdown
Contributor Author

karajan1001 commented May 19, 2020

@jorgeorpinel
@PseudoNerd

Not a native English speaker, and never wrote front-end code. Actually, I can't do much more than now. And If you had any change of the doc of help, I would synchronize it to the related PR

@jorgeorpinel
Copy link
Copy Markdown
Contributor

This is perfect @karajan1001! We'll take it form here. Thank you so much.

Comment thread content/docs/command-reference/remote/rename.md Outdated
@jorgeorpinel
Copy link
Copy Markdown
Contributor

jorgeorpinel commented May 19, 2020

@efiop
Copy link
Copy Markdown
Contributor

efiop commented May 20, 2020

@jorgeorpinel That PR is merged now 🙂

@shcheklein

This comment has been minimized.

@shcheklein shcheklein changed the title [WIP] new remote rename command new remote rename command May 21, 2020
@shcheklein
Copy link
Copy Markdown
Contributor

shcheklein commented May 21, 2020

@jorgeorpinel @karajan1001

we should also update (or make sure that they are up to date):

  • bash/zsh autocompletion scripts (DVC core repo)
  • JS highlighters in this repo to add dvc remote rename

@shcheklein shcheklein temporarily deployed to dvc-landing-fix-1301-tiwrekhop May 21, 2020 21:33 Inactive
@karajan1001
Copy link
Copy Markdown
Contributor Author

karajan1001 commented May 22, 2020

@jorgeorpinel @karajan1001

  • bash/zsh autocompletion scripts (DVC core repo)

Thank you @shcheklein, I had opened a new issue treeverse/dvc/issues/3847 in the core repo to do this.

But may I ask a question? I had never seen an autocompletion on any DVC commands and didn't know their existence. How can I make them come into effect?

I am using zsh on both Linux and macOS and installed DVC from pip.

@jorgeorpinel
Copy link
Copy Markdown
Contributor

Hey guys I cherry-picked @karajan1001's commits into my branch for #1283 instead of pushing here.

I also fixed

JS highlighters in this repo to add dvc remote rename

and other general scaffolding stuff to put this doc in the right place. Closing this PR but thanks again!

@efiop
Copy link
Copy Markdown
Contributor

efiop commented May 23, 2020

But may I ask a question? I had never seen an autocompletion on any DVC commands and didn't know their existence. How can I make them come into effect?

@karajan1001 Pip doesn't install those, unfortunately. You can install them manually using this guide https://dvc.org/doc/install/completion#shell-tab-completion . deb/rpm/brew packages automatically install it though.

@shcheklein
Copy link
Copy Markdown
Contributor

@jorgeorpinel let's merge this and do improvements on top? It's not good that we don't attribute commits and contributions properly this way.

@efiop
Copy link
Copy Markdown
Contributor

efiop commented May 23, 2020

@shcheklein cherry-picking preserves the author of commits. But yeah, shame that this PR wasn't preserved. Though it is a common practice in many projects.

@shcheklein
Copy link
Copy Markdown
Contributor

@efiop @jorgeorpinel yep, I didn't check that Jorge did a cherry-pick. I think it's fine then if it's easier to manage the process this way.

@jorgeorpinel
Copy link
Copy Markdown
Contributor

jorgeorpinel commented May 23, 2020

Yes the commits are preserved in the repo's history (unless you sqash the merge which we don't do AFAIK). But you're right it's better for the contributor to have a merged PR. Reopening, we can merge it, the result will be the same in the end!

@jorgeorpinel jorgeorpinel reopened this May 23, 2020
@jorgeorpinel
Copy link
Copy Markdown
Contributor

However let's wait for mine to merge first? It already has all the improvements to this 👍

Added a checkbox in #1283 (comment)

@shcheklein
Copy link
Copy Markdown
Contributor

@jorgeorpinel feel free to merge in any order :) I think both are approved?

@karajan1001
Copy link
Copy Markdown
Contributor Author

karajan1001 commented May 24, 2020

@jorgeorpinel @efiop @shcheklein
Thank all of you. It's all OK. I am not familiar with front-end, and can't finish this by myself.

@jorgeorpinel jorgeorpinel merged commit 5f1c002 into treeverse:master May 24, 2020
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.

cmd ref: document new subcommand remote rename

4 participants