Skip to content
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

readme: mention vs code extension #7859

Merged
merged 4 commits into from Jun 13, 2022
Merged

readme: mention vs code extension #7859

merged 4 commits into from Jun 13, 2022

Conversation

dberenbaum
Copy link
Contributor

Adds a couple of brief references to the VS Code extension.

Questions/issues to consider:

  • Where (else) in the readme should we mention the extension?
  • Should we be giving any kind of summary or additional info about the extension in the DVC readme?
  • The quickstart could be moved above "How DVC works", which would move up the reference to the extension. We should rewrite/reorganize the rest of the readme, but I didn't want to introduce unrelated changes in this PR.
  • Studio might also be mentioned more and earlier (for example, when discussing sharing and comparing experiments), but this can be a separate discussion.

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

A few ideas:

  • add badge (instead of donate - we don't need it and it's not very useful anyway)
  • add link (section before badges)
  • add a link in the line 26
  • even more extreme add "in CLI (or VS code)" to the line 17
  • mention in the installation section

@dberenbaum
Copy link
Contributor Author

  • add badge (instead of donate - we don't need it and it's not very useful anyway)

Removed donate mentions in a separate PR: #7862.

Did you have a particular badge in mind? For now, I added the version badge from https://github.com/cssho/VSMarketplaceBadge.

  • add link (section before badges)

👍

  • add a link in the line 26

Added for now. I worry the intro and "Why DVC?" become redundant. I would like to combine into a single bullet list in a separate PR.

  • even more extreme add "in CLI (or VS code)" to the line 17

Take a look if this is what you meant. Again, I worry a little about being too redundant. I think we should decide between adding it here or adding it in the bullets.

  • mention in the installation section

See what you think.

README.rst Outdated

|CI| |Maintainability| |Coverage| |Donate| |DOI|
|CI| |Maintainability| |Coverage| |Donate| |VSCode| |DOI|
Copy link
Member

Choose a reason for hiding this comment

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

VS Code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't accept whitespace. It's not visible anyway on the page, so I don't think it's an issue. Let me know if you think it's a problem.

@shcheklein
Copy link
Member

It looks good to me, one small typo, otherwise I think it's good to go. I don't have any other major ideas and not sure anything else is needed.

I think we should decide between adding it here or adding it in the bullets.

I think it's fine to keep it redundant for now (for a few weeks) and we can cleanup it after.

@dberenbaum
Copy link
Contributor Author

@shcheklein @jendefig Are there any important considerations for when this should be merged?

@shcheklein
Copy link
Member

@dberenbaum let's merge it Monday end of the day? (there is a merge conflict, and could you please resolve the open conversations - just to make sure that you saw the feedback :) ).

@dberenbaum dberenbaum force-pushed the vscode branch 2 times, most recently from f8691dc to ae4a263 Compare June 9, 2022 21:01
@dberenbaum
Copy link
Contributor Author

Well I forgot this is rst and not md 🤦 . Please hold.

@dberenbaum dberenbaum marked this pull request as ready for review June 13, 2022 18:55
@dberenbaum dberenbaum requested a review from a team as a code owner June 13, 2022 18:55
@dberenbaum dberenbaum requested a review from dtrifiro June 13, 2022 18:55
README.rst Outdated Show resolved Hide resolved
@dberenbaum
Copy link
Contributor Author

@shcheklein tldr changed to VS Code (I mistakenly thought rst couldn't handle whitespace there).

@efiop efiop merged commit e7b265a into main Jun 13, 2022
@efiop efiop deleted the vscode branch June 13, 2022 21:18
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.

None yet

3 participants