Skip to content

Conversation

srinivasayush
Copy link
Contributor

@srinivasayush srinivasayush commented Jun 21, 2021

Note: This pull request is not trying to make all current and future contributors switch to using VSCode. It just makes it easier for developers already using VSCode to set up, develop, and run gurkbot

  • Removed .vscode entry from .gitignore file
  • Set configuration options for recommended extensions, workspace python path, and VSCode tasks
  • Updated README.md(click to view changes) with development environment setup instructions for VSCode

@srinivasayush srinivasayush marked this pull request as draft June 22, 2021 02:20
@srinivasayush srinivasayush marked this pull request as ready for review June 22, 2021 06:42
@srinivasayush srinivasayush marked this pull request as draft June 22, 2021 20:50
@srinivasayush srinivasayush marked this pull request as ready for review June 27, 2021 15:37
Copy link
Contributor

@Arnav-2004 Arnav-2004 left a comment

Choose a reason for hiding this comment

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

Hi @DudeBro249 , thanks for the PR! Just a few changes.

@srinivasayush
Copy link
Contributor Author

@Arnav-2004 do you think it would be worth putting a pip install -U poetry in the Setup task?

@Arnav-2004
Copy link
Contributor

@Arnav-2004 do you think it would be worth putting a pip install -U poetry in the Setup task?

Yea, probably a good idea.

Copy link
Contributor

@Arnav-2004 Arnav-2004 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dawnofmidnight
Copy link

dawnofmidnight commented Jun 29, 2021

@Arnav-2004 do you think it would be worth putting a pip install -U poetry in the Setup task?

pip is not recommended for installing poetry, for reasons detailed on the installation page. Instead, I think you should link the Poetry installation guide. https://python-poetry.org/docs/#installation

@RohanJnr
Copy link
Member

RohanJnr commented Jun 29, 2021

how are we going to ignore use generated settings.json file? as vscode tends to generate one/users like to have one for config or is it already handled?

@gustavwilliam
Copy link
Member

@DudeBro249 I'm fine with this. If you address the feedback from and get an approval from @RohanJnr, I'll be pretty comfortable merging this. Just test it please.

@srinivasayush
Copy link
Contributor Author

srinivasayush commented Jul 4, 2021

@Arnav-2004 do you think it would be worth putting a pip install -U poetry in the Setup task?

pip is not recommended for installing poetry, for reasons detailed on the installation page. Instead, I think you should link the Poetry installation guide. https://python-poetry.org/docs/#installation

I'll remove the pip install poetry command in the task and link the poetry installation instructions in the README.
(EDIT) Done 👍

@srinivasayush
Copy link
Contributor Author

srinivasayush commented Jul 4, 2021

how are we going to ignore use generated settings.json file? as vscode tends to generate one/users like to have one for config or is it already handled?

I'll look into this. Could you clarify what you mean by this question though?

@srinivasayush
Copy link
Contributor Author

@DudeBro249 I'm fine with this. If you address the feedback from and get an approval from @RohanJnr, I'll be pretty comfortable merging this. Just test it please.

I've tested it and it works. I'll make the necessary changes though.

@srinivasayush srinivasayush marked this pull request as draft July 17, 2021 23:30
@onerandomusername
Copy link
Contributor

Please don't add this. I love configuring my .vscode/settings.json per project myself, and would appreciate it not being pre-configured.

If we want something like this, we can store them elsewhere, and have a script to copy the files to the correct location, but please do not check files in to the repository where per-user configuration is stored.

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.

6 participants