-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Adding devcontainer for easy Python development #1184
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
Conversation
scorphus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
Would you be willing to separate the two features into two different commits and — if it's not too much to ask — two different PR's?
Thanks!
|
Also, the fix for Python 2.7 tests is already on current |
|
Howdy @storey247! Thanks for contributing. I'd really love to read your comments on my suggestions/questions above. I understand that you're probably busy with many stuff. But I thought I'd ping you 🙂 |
|
hi @scorphus thanks for all the feedback, yes I will action and get this PR split into two 😄 apologies for the delayed response, work is a bit crazy atm |
|
Awesome! I already split the commit into three, so no need to create another PR, I can merge the changes regarding the new rule and we leave this PR for the devcontainer addition. What do you think? |
|
Amazing! Nice work! 👏👏👏 sounds like a plan, I’ll rebase this on Monday when the work is merged and I’ll tidy up the container so it’s nice and trim. Thanks @scorphus |
|
There you go, @storey247. Sorry for the extra noise. To continue your work, I'd suggest you force-resetting to the |
|
@scorphus I have now rebased my work from latest I also added some notes into the Hopefully now this work can be merged in too. Let me know if there is anything else you need me to look at |
scorphus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thats super, @storey247! Thanks so much for hanging in! Please consider my suggestion below.
Feedback actioned, just waiting on the builds and then should be good to merge 👍 |
scorphus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @storey247! That's a great contribution!
Also, thanks for the new git_main_master rule! 🙌
Love the tool, wanted to contribute and here is the result :)
mainand some usemasteras the primary integration branchFeedback welcome and thanks for the great tooling