Skip to content

Comments

Adding devcontainer for easy Python development#1184

Merged
scorphus merged 8 commits intonvbn:masterfrom
storey247:main_master
Jul 19, 2021
Merged

Adding devcontainer for easy Python development#1184
scorphus merged 8 commits intonvbn:masterfrom
storey247:main_master

Conversation

@storey247
Copy link
Contributor

@storey247 storey247 commented Apr 6, 2021

Love the tool, wanted to contribute and here is the result :)

  • Added a VSCode devcontainer to help people get up and running with the repo quicker without needing to install any tooling or do any setup of venv
  • Added a new rule to help when some repositories use main and some use master as the primary integration branch

Feedback welcome and thanks for the great tooling

@storey247 storey247 changed the title Adding devcontainer and new rule for main-master Adding devcontainer and new rule for main<->master Apr 20, 2021
scorphus
scorphus previously requested changes Jun 30, 2021
View reviewed changes
Copy link
Collaborator

@scorphus scorphus left a 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!

@scorphus
Copy link
Collaborator

scorphus commented Jun 30, 2021

Also, the fix for Python 2.7 tests is already on current master, would you please rebase on top of that? 🙂

scorphus pushed a commit to storey247/thefuck that referenced this pull request Jul 9, 2021
@storey247 @scorphus
nvbn#1184: Add new rule for main / master Git branches
227e109
scorphus added a commit to storey247/thefuck that referenced this pull request Jul 9, 2021
@scorphus
nvbn#1184: Improve + fix git_main_master rule
03730a6
scorphus pushed a commit to storey247/thefuck that referenced this pull request Jul 9, 2021
@storey247 @scorphus
nvbn#1184: Add devcontainer
a783fa5
@scorphus scorphus dismissed their stale review July 9, 2021 14:15

Requested changes were applied

@scorphus
Copy link
Collaborator

scorphus commented Jul 15, 2021 •
edited
Loading

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 🙂

@storey247
Copy link
Contributor Author

storey247 commented Jul 16, 2021

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

@scorphus
Copy link
Collaborator

scorphus commented Jul 16, 2021

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?

@storey247
Copy link
Contributor Author

storey247 commented Jul 17, 2021

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

scorphus pushed a commit to storey247/thefuck that referenced this pull request Jul 18, 2021
@storey247 @scorphus
nvbn#1184: Add new rule for main / master Git branches
b02ced9
scorphus added a commit to storey247/thefuck that referenced this pull request Jul 18, 2021
@scorphus
nvbn#1184: Improve + fix git_main_master rule
6e3aebd
scorphus pushed a commit to storey247/thefuck that referenced this pull request Jul 18, 2021
@storey247 @scorphus
nvbn#1184: Add devcontainer
32e1367
scorphus pushed a commit to storey247/thefuck that referenced this pull request Jul 18, 2021
@storey247 @scorphus
nvbn#1184: Add new rule for main / master Git branches
70a42b5
scorphus added a commit to storey247/thefuck that referenced this pull request Jul 18, 2021
@scorphus
nvbn#1184: Improve + fix git_main_master rule
711feb4
scorphus pushed a commit to storey247/thefuck that referenced this pull request Jul 18, 2021
@storey247 @scorphus
nvbn#1184: Add devcontainer
837f756
scorphus pushed a commit to storey247/thefuck that referenced this pull request Jul 18, 2021
@storey247
nvbn#1184: Add devcontainer
7c1d535
@scorphus
Copy link
Collaborator

scorphus commented Jul 18, 2021

There you go, @storey247. Sorry for the extra noise. To continue your work, I'd suggest you force-resetting to the main_master branch on your remote, that way you'll have exactly what this PR currently has. Please don't hesitate to ask me for help with that.

@storey247 storey247 changed the title Adding devcontainer and new rule for main<->master Adding devcontainer for easy Python development Jul 19, 2021
@storey247
Copy link
Contributor Author

storey247 commented Jul 19, 2021

@scorphus I have now rebased my work from latest master and cleaned up the comments as discussed.

I also added some notes into the Contribute.md to help people get up and running with the devcontainer setup if required.

Hopefully now this work can be merged in too. Let me know if there is anything else you need me to look at

scorphus
scorphus approved these changes Jul 19, 2021
View reviewed changes
Copy link
Collaborator

@scorphus scorphus left a 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.

@storey247
Copy link
Contributor Author

storey247 commented Jul 19, 2021 •
edited
Loading

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
scorphus approved these changes Jul 19, 2021
View reviewed changes
Copy link
Collaborator

@scorphus scorphus left a 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! 🙌

@scorphus scorphus merged commit 58f61d8 into nvbn:master Jul 19, 2021
@storey247 storey247 deleted the main_master branch July 19, 2021 21:27
scorphus added a commit to scorphus/thefuck that referenced this pull request Jul 21, 2021
@scorphus
nvbn#1184: Configure devcontainer shell with recommended way
98db80a 
https://code.visualstudio.com/docs/editor/integrated-terminal#_configuration
scorphus added a commit that referenced this pull request Jul 29, 2021
@scorphus
#1184: Configure devcontainer shell with recommended way
30c90bc 
https://code.visualstudio.com/docs/editor/integrated-terminal#_configuration
@scorphus scorphus added this to the 3.32 milestone Sep 8, 2021
@scorphus scorphus linked an issue Sep 8, 2021 that may be closed by this pull request
Feature request: master to main #1234
Closed
@scorphus scorphus mentioned this pull request Sep 8, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scorphus scorphus scorphus approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

3.32

Development

Successfully merging this pull request may close these issues.

Feature request: master to main

2 participants

@storey247 @scorphus