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.

Sign up for GitHub

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

Add VSCode folder to gitignore #255

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+3 −0
Open

Add VSCode folder to gitignore #255

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f51b02
Add basic settings.json file
MayberryZoom Dec 22, 2024
3abd83d
Ignore vscode folder
MayberryZoom Dec 22, 2024
09d84e7
Delete .vscode/settings.json
MayberryZoom Dec 23, 2024
e8e09a9
Merge branch 'main' into vscode
MayberryZoom Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ dmypy.json
# PyCharm
/.idea

# VsCode
/.vscode

# Generated files
src/mercury_engine_data_structures/version.py
src/mercury_engine_data_structures/formats/dread_types.py
Expand Down
6 changes: 6 additions & 0 deletions .vscode/settings.json
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

All lines besides "python.testing.pytestEnabled": true, are the default values.
Any reason why they are added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that it works out of the box.

Copy link
Contributor

Choose a reason for hiding this comment

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

if the values are default they should work out of the box, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure the env file isn't default anyways. The rest is auto-generated by VSCode but without declaring the env file it won't work.

Copy link
Contributor

Choose a reason for hiding this comment

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

The path for the env file is the default.

Copy link
Contributor

Choose a reason for hiding this comment

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

I really can't follow.

I'm pretty sure the pytest enabled setting is added automatically by VSCode when you press the configure tests button then select pytest

That is not

So that it works out of the box.

That is why suggested to

At least values which are different from the default should stay in the file which is only "python.testing.pytestEnabled": true

So, why is it now completely removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you open up the project in VSCode and go to the tests side bar, if there's no settings.json it will prompt you to configure tests. If you press that button you can select pytest, then it auto generates the settings.json file. That "python.testing.pytestEnabled": true is part of that file. And as you said, so is the env path. So imo there's no need to include that file in the repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Anything holding this pr back?

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically me who still doesn't know what's the goal here. If it is to just ignore the settings.json, it's fine.
If it is to work "out of the box", then it's missing stuff.
As already described, without a settings.json, VSCode will prompt you to configure tests but that's a selection between two different frameworks where the wrong one is even described as Standard Python test framework
image

Eventually there should be at least an example .env file in the README or a .env file with placeholders?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, it'd be nice to figure out so I don't have to keep a settings.json file with changes on every branch I work on.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"python.testing.pytestArgs": [],
"python.testing.unittestEnabled": false,
"python.testing.pytestEnabled": true,
"python.envFile": "${workspaceFolder}/.env"
}
Loading