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

docs: README updates VSCODE-395 #516

Merged
merged 8 commits into from
May 15, 2023
Merged

docs: README updates VSCODE-395 #516

merged 8 commits into from
May 15, 2023

Conversation

mmarcon
Copy link
Member

@mmarcon mmarcon commented Apr 25, 2023

Description

  • Updates to README content
  • Removal of preview flag

Checklist

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

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

Choose a reason for hiding this comment

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

Do we want to use the use the db.getCollection('collection_name').aggregate([... format here in the future?
We use it in the default template:

db.getCollection('sales').insertMany([

#470 (comment)
Since the gifs are already made if it's a lot of time to remake them maybe it's not worth updating.

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer not to redo the gifs 😄

Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

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

The table for settings looks real nice. And the preview is gone 🙌
Nice updates.

README.md Outdated Show resolved Hide resolved

- Prototype your queries, aggregations, and MongoDB commands with convenient syntax highlighting and intelligent autocomplete for MongoDB shell API, BSON types, MQL operators, aggregation stage snippets, system variables, and for database, collection, and field names.
- Prototype your queries, aggregations, and MongoDB commands with convenient syntax highlighting and intelligent autocomplete for MongoDB Shell API, BSON types, MongoDB Query API, system variables, and for database, collection, and field names.
- Run your playgrounds and see the results instantly. Click the play button in the tab bar to see the output.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want maybe to mention a shortcut here as well? We have only two of them to run all and to run partial lines of a playground. From time to time people open GitHub issues about shortcuts, so we could list them for clarity. We could also say that they can reassign shortcuts according to their preferences by opening Settings... -> Keyboars Shortcuts, checking the currently available shortcuts, searching for the command they want to assign a shortcut to, and picking the shortcut for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

As an alternative, we could move the shortcuts list to a separate section and show them in a table view as you did for settings.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated

![Settings](resources/screenshots/settings.png)

| Setting | Description | Default |
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could reformat the table to optimize the columns' width. The text is squished in the middle row.

Copy link
Member Author

Choose a reason for hiding this comment

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

Column width is not easy to control in markdown tables. Let me see how it looks if I add a new line for the longer settings.

@alenakhineika
Copy link
Contributor

I think because of gif files the bundle size has grown significantly from the expected max 7000000 bytes, it got 10718906. Should we store them somewhere else maybe?

@mmarcon
Copy link
Member Author

mmarcon commented May 15, 2023

Those go into the bundle?! Ok, then we should probably store them somewhere else. Any idea where? We could make a gh-pages branch and store them in github pages. Let me think if we have other options. S3 could also be an option but it's not the easiest place to maintain afterwards.

@mmarcon
Copy link
Member Author

mmarcon commented May 15, 2023

Looking at how the images are served in the marketplace, we should be able to safely exclude the resources folder in .vscodeignore.

image

@alenakhineika can we test that out somehow?

@alenakhineika
Copy link
Contributor

You can also add those to .vscodeignore and they should be excluded from the bundle.

@alenakhineika
Copy link
Contributor

alenakhineika commented May 15, 2023

Just have seen your last comment, yes! :) To check you can npm run local-install and see what is inside ~/alena.khineika/.vscode/extensions/mongodb.mongodb-vscode-0.0.0-dev.0. Gifs are not supposed to be there. Also, your tests will pass since they check the bundle size. But if the marketplace shows readme from the bundle, the links to gifs should be an absolute path to GitHub resources.

@mmarcon
Copy link
Member Author

mmarcon commented May 15, 2023

Ok cool. And then we keep our fingers crossed and hope the images will be there in the marketplace?

Massimiliano Marcon added 2 commits May 15, 2023 09:14
Looks like most of the extensions about databases use it
singular vs plural.
Using HTML on purpose to center the banner.

This means I need to use an `img` tag and use the absolute URL because
I doubt the marketplace gets it replaced when it's not in markdown.

The banner will be visible in the README once the PR is merged.
@mmarcon
Copy link
Member Author

mmarcon commented May 15, 2023

@Anemy if you are cool with this, I'll merge.

@mmarcon mmarcon merged commit 937543f into main May 15, 2023
@mmarcon mmarcon deleted the VSCODE-395 branch May 15, 2023 13:22
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.

3 participants