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

Fix: Handle case where Python version is already installed (#222) #1115

Merged

Conversation

gvatsal60
Copy link
Contributor

@gvatsal60 gvatsal60 commented Sep 5, 2024

@gvatsal60 gvatsal60 requested a review from a team as a code owner September 5, 2024 06:24
@gvatsal60
Copy link
Contributor Author

Hi @samruddhikhandale
Could you check out the pull request when you have a moment?

@samruddhikhandale
Copy link
Member

@gvatsal60 The code is intentionally in place. Why would you want to remove exit 1? If the version still exists, why not pass none or another version?

@gvatsal60
Copy link
Contributor Author

@gvatsal60 The code is intentionally in place. Why would you want to remove exit 1? If the version still exists, why not pass none or another version?

I am looking into it. Will update the PR soon.

@gvatsal60
Copy link
Contributor Author

@samruddhikhandale I have updated the PR. Please have a look...

Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

This change make sense to me, let's merge provided the tests are green. Thanks!

@samruddhikhandale samruddhikhandale merged commit d231662 into devcontainers:main Oct 8, 2024
12 checks passed
@rubensa
Copy link
Contributor

rubensa commented Oct 10, 2024

@samruddhikhandale Shouldn't this have generated a version update?

I thought that any feature change merged to main should have its corresponding version update.

NOTE: I just came here trying to check if this PR made the GitHub tags work again but, as the version was not updated, no luck to check...

@gvatsal60
Copy link
Contributor Author

Hi @rubensa,

I realized I forgot to update the version in the previous PR. I've submitted a new one to address this: #1148.

I’ll also look into solutions to prevent version update oversights in the future.

Thank you!

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.

4 participants