Skip to content

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 gvatsal60 requested a review from kiseCrow October 2, 2024 03:14
@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