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

Include workspace recommendations for extensions in VS code #5691

Open
wants to merge 1 commit into
base: develop
from

Conversation

@andrew-codes
Copy link
Contributor

andrew-codes commented Nov 13, 2019

Closes #5689

This will enable showing/sharing recommended VS Code extensions for developers working on Cypress. Several have been added as a starting place. This is accessed in VS Code via searching for @recommended extensions under the Workspace Recommended Extensions section.

image

@cypress-bot

This comment has been minimized.

Copy link

cypress-bot bot commented Nov 13, 2019

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

  • Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
  • Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

  • The feature/bugfix is self-documenting from within the product.
  • The change provides the end user with a way to fix their problem (no dead ends).

Functionality

  • The code works and performs its intended function with the correct logic.
  • Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
  • The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

  • The code is readable (too many nested 'if's are a bad sign).
  • Names used for variables, methods, etc, clearly describe their function.
  • The code is easy to understood and there are relevant comments explaining.
  • New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
  • There are comments containing link(s) to the addressed issue (in tests and code).

Quality

  • The change does not reimplement code.
  • There's not a module from the ecosystem that should be used instead.
  • There is no redundant or duplicate code.
  • There are no irrelevant comments left in the code.
  • Tests are testing the code’s intended functionality in the best way possible.

Internal

  • The original issue has been tagged with a release in ZenHub.
@andrew-codes andrew-codes self-assigned this Nov 13, 2019
@andrew-codes andrew-codes requested a review from brian-mann Nov 13, 2019
@cypress

This comment has been minimized.

Copy link

cypress bot commented Nov 13, 2019



Test summary

3504 0 45 0


Run details

Project cypress
Status Passed
Commit 6a387aa
Started Nov 13, 2019 5:17 PM
Ended Nov 13, 2019 5:21 PM
Duration 04:17 💡
OS Linux Debian - 9.8
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

"britesnow.vscode-toggle-quotes",
"pflannery.vscode-versionlens"
]
}

This comment has been minimized.

Copy link
@jennifer-shehane

jennifer-shehane Nov 13, 2019

Member

I feel like a lot of these are too opinionated.

We should just recommend the ones that are essential for contributors to commit - like, they need eslint to work, coffescript support utils, test-utils, typescript utils, really any utilities that will catch things before the linter will in CI - cause a lot of them don't check CI later.

I don't use any of these for example and would just be another file that may be confusing to contributors.

This comment has been minimized.

Copy link
@flotwig

flotwig Nov 13, 2019

Member

yeah, i like the idea of recommending extensions, but maybe it should be done via documentation, so we can be like "here's what you NEED. here's what you might want."

This comment has been minimized.

Copy link
@andrew-codes

andrew-codes Nov 19, 2019

Author Contributor

The intention was more to share recommended extensions with our team. It doesn't auto-install them or gets in the way (merge conflicts, etc.). But it does make it easier to discover and also install them. It is not much different than documentation, but this provides more value than documentation alone. With that said, I'm not opposed to adding it to the docs if it means it getting in or not getting in; I'm just not sure the difference between the two is worth the value trade-off.

I'm hardpressed to believe that contributors will find this confusing. I also doubt that the number of contributors to add to this would be overwhelming for us; issues/PRs opened, etc. Sure, I could be wrong about this, but I'm not convinced as of now this is the case and I'm unsure if anyone would be able to definitively say it is one way or another. Due to us simply not knowing, I'd rather add value and iterate on it later (remove it, etc.) than simply never add it because we think it could be a problem in a nebulous way.

I suppose if the list were to grow to an enormous number then that could be problematic, but that is a future problem that we may never even encounter. I doubt the list will continue to grow at a steady rate over time. I imagine there is a rough limit that we will naturally arrive at that is not unmanageable.

This comment has been minimized.

Copy link
@jennifer-shehane

jennifer-shehane Nov 19, 2019

Member

If this is for our team only, then there should be some discussion on the extensions that are essential for our team, because this list seems very arbitrary to me. I only use 2 of these extensions for example, so this just seems like your personal list of extensions which is just noise in our repo if no one ever uses.

This comment has been minimized.

Copy link
@andrew-codes

andrew-codes Nov 19, 2019

Author Contributor

I can agree with that for sure. These are some that I have found helpful and/or learned about using with the Cypress codebase. What are the two you use that are also on the list? We can add/remove as necessary. Also, are there any that you find helpful that are not on the list?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.